Implement library for topology mutation hooks implementation

Author: fabriziopandini

docs/book/src/tasks/experimental-features/runtime-sdk/implement-extensions.md

@@ -56,8 +56,8 @@ import (
 )
 
 var (
-	catalog  = runtimecatalog.New()
-	setupLog = ctrl.Log.WithName("setup")
+	// catalog contains all information about RuntimeHooks.
+	catalog = runtimecatalog.New()
 
 	// Flags.
 	profilerAddress string
@@ -67,15 +67,17 @@ var (
 )
 
 func init() {
-	// Register the Runtime Hook types into the catalog.
+	// Adds to the catalog all the RuntimeHooks defined in cluster API.
 	_ = runtimehooksv1.AddToCatalog(catalog)
 }
 
 // InitFlags initializes the flags.
 func InitFlags(fs *pflag.FlagSet) {
+	// Initialize logs flags using Kubernetes component-base machinery.
 	logs.AddFlags(fs, logs.SkipLoggingConfigurationFlags())
 	logOptions.AddFlags(fs)
 
+	// Add test-extension specific flags
 	fs.StringVar(&profilerAddress, "profiler-address", "",
 		"Bind address to expose the pprof profiler (e.g. localhost:6060)")
 
@@ -87,29 +89,34 @@ func InitFlags(fs *pflag.FlagSet) {
 }
 
 func main() {
+	// Creates a logger to be used during the main func.
+	setupLog := ctrl.Log.WithName("main")
+
+	// Initialize and parse command line flags.
 	InitFlags(pflag.CommandLine)
 	pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc)
 	pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
 	pflag.Parse()
 
+	// Validates logs flags using Kubernetes component-base machinery and applies them
 	if err := logOptions.ValidateAndApply(nil); err != nil {
 		setupLog.Error(err, "unable to start extension")
 		os.Exit(1)
 	}
 
-	// klog.Background will automatically use the right logger.
+	// Add the klog logger in the context.
 	ctrl.SetLogger(klog.Background())
 
+	// Initialize the golang profiler server, if required.
 	if profilerAddress != "" {
 		klog.Infof("Profiler listening for requests at %s", profilerAddress)
 		go func() {
 			klog.Info(http.ListenAndServe(profilerAddress, nil))
 		}()
 	}
 
-	ctx := ctrl.SetupSignalHandler()
-
-	webhookServer, err := server.NewServer(server.Options{
+	// Create a http server for serving runtime extensions
+	webhookServer, err := server.New(server.Options{
 		Catalog: catalog,
 		Port:    webhookPort,
 		CertDir: webhookCertDir,
@@ -124,8 +131,6 @@ func main() {
 		Hook:           runtimehooksv1.BeforeClusterCreate,
 		Name:           "before-cluster-create",
 		HandlerFunc:    DoBeforeClusterCreate,
-		TimeoutSeconds: pointer.Int32(5),
-		FailurePolicy:  toPtr(runtimehooksv1.FailurePolicyFail),
 	}); err != nil {
 		setupLog.Error(err, "error adding handler")
 		os.Exit(1)
@@ -134,13 +139,15 @@ func main() {
 		Hook:           runtimehooksv1.BeforeClusterUpgrade,
 		Name:           "before-cluster-upgrade",
 		HandlerFunc:    DoBeforeClusterUpgrade,
-		TimeoutSeconds: pointer.Int32(5),
-		FailurePolicy:  toPtr(runtimehooksv1.FailurePolicyFail),
 	}); err != nil {
 		setupLog.Error(err, "error adding handler")
 		os.Exit(1)
 	}
 
+	// Setup a context listening for SIGINT.
+	ctx := ctrl.SetupSignalHandler()
+
+	// Start the https server.
 	setupLog.Info("Starting Runtime Extension server")
 	if err := webhookServer.Start(ctx); err != nil {
 		setupLog.Error(err, "error running webhook server")
@@ -159,10 +166,6 @@ func DoBeforeClusterUpgrade(ctx context.Context, request *runtimehooksv1.BeforeC
 	log.Info("BeforeClusterUpgrade is called")
 	// Your implementation
 }
-
-func toPtr(f runtimehooksv1.FailurePolicy) *runtimehooksv1.FailurePolicy {
-	return &f
-}
 ```
 
 For a full example see our [test extension](https://github.com/kubernetes-sigs/cluster-api/tree/main/test/extension).
@@ -270,7 +273,26 @@ Some Runtime Hooks, e.g. like external patches, might explicitly request for cor
 to support this property. But we encourage developers to follow this pattern more generally given that it fits
 well with practices like unit testing and generally makes the entire system more predictable and easier to troubleshoot.
 
-### Error Management
+### Error messages
+
+RuntimeExtension authors should be aware that error messages are surfaced as a conditions in Kubernetes resources 
+and recorded in Cluster API controller's logs. As a consequence:
+
+- Error message must not contain any sensitive information.
+- Error message must be deterministic, and must avoid to including timestamps or values changing at every call.
+- Error message must not contain external errors when it's not clear if those errors are deterministic (e.g. errors return from cloud APIs).
+
+<aside class="note warning">
+
+<h1>Caution</h1>
+
+If an error message is not deterministic and it changes at every call even if the problem is the same, it could
+lead to to Kubernetes resources conditions continuously changing, and this generates a denial attack to 
+controllers processing those resource that might impact system stability.
+
+</aside>
+
+### Error management
 
 In case a Runtime Extension returns an error, the error will be handled according to the corresponding failure policy
 defined in the response of the Discovery call.

docs/book/src/tasks/experimental-features/runtime-sdk/implement-lifecycle-hooks.md

@@ -29,6 +29,7 @@ potentially block lifecycle transitions from happening.
 Following recommendations are especially relevant:
 
 * [Blocking and non Blocking](implement-extensions.md#blocking-hooks)
+* [Error messages](implement-extensions.md#error-messages)
 * [Error management](implement-extensions.md#error-management)
 * [Avoid dependencies](implement-extensions.md#avoid-dependencies)
 

docs/book/src/tasks/experimental-features/runtime-sdk/implement-topology-mutation-hook.md

@@ -67,6 +67,9 @@ This section outlines considerations specific to Topology Mutation hooks:
   i.e. unnecessary patches when the template is already in the desired state must be avoided.
 * **Avoid Dependencies**: An External Patch Extension must be independent of other External Patch Extensions. However
   if dependencies cannot be avoided, it is possible to control the order in which patches are executed via the ClusterClass.
+* **Error messages**: For a given request (a set of templates and variables) an External Patch Extension must
+  always return the same error message. Otherwise the system might became unstable due to controllers being overloaded
+  by continuous changes to Kubernetes resources as these messages are reported as conditions. See [error messages](implement-extensions.md#error-messages).
 
 ## Definitions
 

exp/runtime/hooks/api/v1alpha1/discovery_types.go

@@ -22,6 +22,9 @@ import (
 	runtimecatalog "sigs.k8s.io/cluster-api/exp/runtime/catalog"
 )
 
+// DefaultHandlersTimeoutSeconds defines the default timeout duration for client calls to ExtensionHandlers.
+const DefaultHandlersTimeoutSeconds = 10
+
 // DiscoveryRequest is the request of the Discovery hook.
 // +kubebuilder:object:root=true
 type DiscoveryRequest struct {

exp/runtime/server/server.go

@@ -71,7 +71,14 @@ type Options struct {
 }
 
 // NewServer creates a new runtime webhook server based on the given Options.
+//
+// Deprecated: use New instead.
 func NewServer(options Options) (*Server, error) {
+	return New(options)
+}
+
+// New creates a new runtime webhook server based on the given Options.
+func New(options Options) (*Server, error) {
 	if options.Catalog == nil {
 		return nil, errors.Errorf("catalog is required")
 	}
@@ -118,9 +125,13 @@ type ExtensionHandler struct {
 	HandlerFunc runtimecatalog.Hook
 
 	// TimeoutSeconds is the timeout of the extension handler.
+	// If left undefined, this will be defaulted to 10s when processing the answer to the discovery
+	// call for this server.
 	TimeoutSeconds *int32
 
-	// FailurePolicy is the failure policy of the extension handler
+	// FailurePolicy is the failure policy of the extension handler.
+	// If left undefined, this will be defaulted to FailurePolicyFail when processing the answer to the discovery
+	// call for this server.
 	FailurePolicy *runtimehooksv1.FailurePolicy
 }
 

exp/runtime/topologymutation/doc.go

@@ -0,0 +1,18 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package topologymutation provides helpers for implementing the topology mutation hooks.
+package topologymutation

exp/runtime/topologymutation/logRef.go

@@ -0,0 +1,46 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package topologymutation
+
+import "strings"
+
+// logRef is used to correctly render a reference with GroupVersionKind, Namespace and Name for both JSON and text logging.
+type logRef struct {
+	Group     string `json:"group,omitempty"`
+	Version   string `json:"version,omitempty"`
+	Kind      string `json:"kind,omitempty"`
+	Namespace string `json:"namespace,omitempty"`
+	Name      string `json:"name,omitempty"`
+}
+
+func (l logRef) String() string {
+	var parts []string
+	for _, s := range []string{l.Group, l.Version, l.Kind, l.Namespace, l.Name} {
+		if strings.TrimSpace(s) != "" {
+			parts = append(parts, s)
+		}
+	}
+	return strings.Join(parts, "/")
+}
+
+type logRefWithoutStringFunc logRef
+
+// MarshalLog ensures that loggers with support for structured output will log
+// as a struct by removing the String method via a custom type.
+func (l logRef) MarshalLog() interface{} {
+	return logRefWithoutStringFunc(l)
+}

exp/runtime/topologymutation/logRef_test.go

@@ -0,0 +1,38 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package topologymutation
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+)
+
+func Test_Log(t *testing.T) {
+	g := NewWithT(t)
+
+	l := &logRef{
+		Group:     "group",
+		Version:   "version",
+		Kind:      "kind",
+		Namespace: "namespace",
+		Name:      "name",
+	}
+
+	g.Expect(l.String()).To(Equal("group/version/kind/namespace/name"))
+	g.Expect(l.MarshalLog()).To(Equal(logRefWithoutStringFunc(*l)))
+}

exp/runtime/topologymutation/variables.go

@@ -0,0 +1,55 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package topologymutation
+
+import (
+	"strconv"
+
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+
+	patchvariables "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/variables"
+)
+
+// TODO: Add func for validating received variables are of the expected types
+// TODO: Add func for converting complex variables into go structs and/or other basic types.
+
+// GetVariable get the variable value.
+func GetVariable(templateVariables map[string]apiextensionsv1.JSON, variableName string) (*apiextensionsv1.JSON, bool, error) {
+	value, err := patchvariables.GetVariableValue(templateVariables, variableName)
+	if err != nil {
+		if patchvariables.IsNotFoundError(err) {
+			return nil, false, nil
+		}
+		return nil, false, err
+	}
+	return value, true, nil
+}
+
+// GetStringVariable get the variable value as a string.
+func GetStringVariable(templateVariables map[string]apiextensionsv1.JSON, variableName string) (string, bool, error) {
+	value, found, err := GetVariable(templateVariables, variableName)
+	if !found || err != nil {
+		return "", found, err
+	}
+
+	// Unquote the JSON string.
+	stringValue, err := strconv.Unquote(string(value.Raw))
+	if err != nil {
+		return "", true, err
+	}
+	return stringValue, true, nil
+}