From 23fe99f1d11df3e2c3689aa5d3bdde036bd448e1 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Tue, 2 Nov 2021 11:23:16 +0100 Subject: [PATCH] fix review findings add unit tests --- .../extensions/patches/api/interface.go | 4 +- .../internal/extensions/patches/engine.go | 195 +------ .../extensions/patches/engine_test.go | 542 ++++++++++++++++++ .../internal/extensions/patches/patch.go | 182 ++++++ .../internal/extensions/patches/patch_test.go | 369 ++++++++++++ .../patches/variables/variables_test.go | 190 ++++++ internal/builder/builders.go | 27 +- 7 files changed, 1322 insertions(+), 187 deletions(-) create mode 100644 controllers/topology/internal/extensions/patches/engine_test.go create mode 100644 controllers/topology/internal/extensions/patches/patch.go create mode 100644 controllers/topology/internal/extensions/patches/patch_test.go create mode 100644 controllers/topology/internal/extensions/patches/variables/variables_test.go diff --git a/controllers/topology/internal/extensions/patches/api/interface.go b/controllers/topology/internal/extensions/patches/api/interface.go index a272dd57884c..d37087608482 100644 --- a/controllers/topology/internal/extensions/patches/api/interface.go +++ b/controllers/topology/internal/extensions/patches/api/interface.go @@ -124,8 +124,8 @@ const ( // JSONPatchType identifies a https://datatracker.ietf.org/doc/html/rfc6902 json patch. JSONPatchType PatchType = 0 - // MergePatchType identifies a https://datatracker.ietf.org/doc/html/rfc7386 json merge patch. - MergePatchType PatchType = 1 + // JSONMergePatchType identifies a https://datatracker.ietf.org/doc/html/rfc7386 json merge patch. + JSONMergePatchType PatchType = 1 ) // GenerateResponse defines the response of a Generate request. diff --git a/controllers/topology/internal/extensions/patches/engine.go b/controllers/topology/internal/extensions/patches/engine.go index a3b77f8cb516..4e5dd945d9cf 100644 --- a/controllers/topology/internal/extensions/patches/engine.go +++ b/controllers/topology/internal/extensions/patches/engine.go @@ -21,7 +21,6 @@ import ( "context" "encoding/json" "fmt" - "strings" jsonpatch "github.com/evanphx/json-patch" "github.com/pkg/errors" @@ -33,7 +32,6 @@ import ( "sigs.k8s.io/cluster-api/controllers/topology/internal/extensions/patches/api" "sigs.k8s.io/cluster-api/controllers/topology/internal/extensions/patches/variables" tlog "sigs.k8s.io/cluster-api/controllers/topology/internal/log" - "sigs.k8s.io/cluster-api/controllers/topology/internal/mergepatch" "sigs.k8s.io/cluster-api/controllers/topology/internal/scope" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -67,20 +65,20 @@ func Apply(ctx context.Context, blueprint *scope.ClusterBlueprint, desired *scop } // Generate patches. - patches, err := generator.Generate(ctx, req) + resp, err := generator.Generate(ctx, req) if err != nil { return errors.Errorf("failed to generate patches for patch %q", patch.Name) } // Apply patches to the request. - if err := applyPatchesToRequest(ctx, req, patches); err != nil { + if err := applyPatchesToRequest(ctx, req, resp); err != nil { return err } } // Use patched templates to update the desired state objects. log.V(5).Infof("Applying patches to desired state") - if err := requestToDesiredState(ctx, req, blueprint, desired); err != nil { + if err := updateDesiredState(ctx, req, blueprint, desired); err != nil { return errors.Wrapf(err, "failed to apply patches to desired state") } @@ -216,7 +214,7 @@ func request(blueprint *scope.ClusterBlueprint, desired *scope.ClusterState) (*a // newTemplate returns a GenerateRequestTemplate object based on the client.Object and the templateID. func newTemplate(obj client.Object, templateID api.TemplateID) (*api.GenerateRequestTemplate, error) { - ret := &api.GenerateRequestTemplate{ + t := &api.GenerateRequestTemplate{ TemplateID: templateID, } @@ -224,9 +222,9 @@ func newTemplate(obj client.Object, templateID api.TemplateID) (*api.GenerateReq if err != nil { return nil, errors.Wrap(err, "failed to marshal object to json") } - ret.Template = apiextensionsv1.JSON{Raw: jsonObj} + t.Template = apiextensionsv1.JSON{Raw: jsonObj} - return ret, nil + return t, nil } // lookupMDTopology looks up the MachineDeploymentTopology based on a mdTopologyName in a topology. @@ -270,9 +268,7 @@ func applyPatchesToRequest(ctx context.Context, req *api.GenerateRequest, resp * // If a patch doesn't apply to any template, this is a misconfiguration. if template == nil { - log.V(5).Infof("Could not find template in GenerateRequest") - // TODO(TBD) @fabrizio should we return an error here? - continue + return errors.Errorf("could not find template with id %q in GenerateRequest", templateIDRef(patch.TemplateID)) } // Use the patch to create a patched copy of the template. @@ -293,7 +289,7 @@ func applyPatchesToRequest(ctx context.Context, req *api.GenerateRequest, resp * return errors.Wrapf(err, "failed to apply patch to template %s: error applying json patch (RFC6902): %s", templateIDRef(template.TemplateID), string(patch.Patch.Raw)) } - case api.MergePatchType: + case api.JSONMergePatchType: log.V(5).Infof("Accumulating JSON merge patch", "patch", string(patch.Patch.Raw)) patchedTemplate, err = jsonpatch.MergePatch(template.Template.Raw, patch.Patch.Raw) if err != nil { @@ -312,10 +308,10 @@ func applyPatchesToRequest(ctx context.Context, req *api.GenerateRequest, resp * return nil } -// requestToDesiredState uses the patched templates of a GenerateRequest to update the desired state. +// updateDesiredState uses the patched templates of a GenerateRequest to update the desired state. // NOTE: This func should be called after all the patches have been applied to the GenerateRequest. -// TODO: Let's only overwrite desired state objects if the corresponding template actually have been patched. -func requestToDesiredState(ctx context.Context, req *api.GenerateRequest, blueprint *scope.ClusterBlueprint, desired *scope.ClusterState) error { +// TODO(sbueringer): Let's only overwrite desired state objects if the corresponding template actually have been patched. +func updateDesiredState(ctx context.Context, req *api.GenerateRequest, blueprint *scope.ClusterBlueprint, desired *scope.ClusterState) error { var err error // Update the InfrastructureCluster. @@ -497,172 +493,3 @@ func bytesToUnstructured(b []byte) (*unstructured.Unstructured, error) { return u, nil } - -// PatchOption represents an option for the patchObject and patchTemplate funcs. -type PatchOption interface { - // ApplyToHelper applies configuration to the given options. - ApplyToHelper(*PatchOptions) -} - -// PatchOptions contains options for patchObject and patchTemplate. -type PatchOptions struct { - preserveFields []contract.Path -} - -// ApplyOptions applies the given patch options -// and then returns itself (for convenient chaining). -func (o *PatchOptions) ApplyOptions(opts []PatchOption) *PatchOptions { - for _, opt := range opts { - opt.ApplyToHelper(o) - } - return o -} - -// PreserveFields instructs the patch func to preserve fields. -type PreserveFields []contract.Path - -// ApplyToHelper applies this configuration to the given patch options. -func (i PreserveFields) ApplyToHelper(opts *PatchOptions) { - opts.preserveFields = i -} - -// patchObject overwrites spec in object with spec.template.spec of patchedTemplate, -// while preserving the configured fields. -func patchObject(ctx context.Context, object, patchedTemplate *unstructured.Unstructured, opts ...PatchOption) error { - log := tlog.LoggerFrom(ctx) - - patchOptions := &PatchOptions{} - patchOptions = patchOptions.ApplyOptions(opts) - - // Create a copy of the object. - original := object.DeepCopy() - - if err := copySpec(copySpecInput{ - src: patchedTemplate, - dest: object, - srcSpecPath: "spec.template.spec", - destSpecPath: "spec", - fieldsToPreserve: patchOptions.preserveFields, - }); err != nil { - return errors.Wrapf(err, "failed to patch object %s by appying changes from the patched template", - tlog.KObj{Obj: object}) - } - - // Log the delta between the object before and after applying the accumulated patches. - helper, err := mergepatch.NewHelper(original, object, nil) - if err != nil { - return err - } - log.V(4).WithObject(object).Infof("Applying accumulated patches", "delta", string(helper.Changes())) - - return nil -} - -// patchTemplate overwrites spec.template.spec in template with spec.template.spec of patchedTemplate, -// while preserving the configured fields. -func patchTemplate(ctx context.Context, template, patchedTemplate *unstructured.Unstructured, opts ...PatchOption) error { - log := tlog.LoggerFrom(ctx) - - patchOptions := &PatchOptions{} - patchOptions = patchOptions.ApplyOptions(opts) - - // Create a copy of the template. - original := template.DeepCopy() - - if err := copySpec(copySpecInput{ - src: patchedTemplate, - dest: template, - srcSpecPath: "spec.template.spec", - destSpecPath: "spec.template.spec", - fieldsToPreserve: patchOptions.preserveFields, - }); err != nil { - return errors.Wrapf(err, "failed to patch template %s by appying changes from the patched template", - tlog.KObj{Obj: template}) - } - - // Log the delta between the object before and after applying the accumulated patches. - helper, err := mergepatch.NewHelper(original, template, nil) - if err != nil { - return err - } - log.V(4).WithObject(template).Infof("Applying accumulated patches", "delta", string(helper.Changes())) - - return nil -} - -// patchTemplateSpec overwrites spec in templateJSON with spec of patchedTemplateBytes. -func patchTemplateSpec(templateJSON *apiextensionsv1.JSON, patchedTemplateBytes []byte) error { - // Convert templates to Unstructured. - template, err := bytesToUnstructured(templateJSON.Raw) - if err != nil { - return errors.Wrap(err, "failed to convert template to Unstructured") - } - patchedTemplate, err := bytesToUnstructured(patchedTemplateBytes) - if err != nil { - return errors.Wrap(err, "failed to convert patched template to Unstructured") - } - - // Copy spec from patchedTemplate to template. - if err := copySpec(copySpecInput{ - src: patchedTemplate, - dest: template, - srcSpecPath: "spec", - destSpecPath: "spec", - }); err != nil { - return errors.Wrap(err, "failed to apply patch to template") - } - - // Marshal template and store it in templateJSON. - templateBytes, err := template.MarshalJSON() - if err != nil { - return errors.Wrapf(err, "failed to marshal patched template") - } - templateJSON.Raw = templateBytes - return nil -} - -type copySpecInput struct { - src *unstructured.Unstructured - dest *unstructured.Unstructured - srcSpecPath string - destSpecPath string - fieldsToPreserve []contract.Path -} - -// copySpec copies a field from a srcSpecPath in src to a destSpecPath in dest, -// while preserving fieldsToPreserve. -func copySpec(in copySpecInput) error { - // Backup fields that should be preserved from dest. - preservedFields := map[string]interface{}{} - for _, field := range in.fieldsToPreserve { - value, found, err := unstructured.NestedFieldNoCopy(in.dest.Object, field...) - if !found { - // Continue if the field does not exist in src. fieldsToPreserve don't have to exist. - continue - } else if err != nil { - return errors.Wrapf(err, "failed to get field %q from %s", strings.Join(field, "."), tlog.KObj{Obj: in.dest}) - } - preservedFields[strings.Join(field, ".")] = value - } - - // Get spec from src. - srcSpec, found, err := unstructured.NestedFieldNoCopy(in.src.Object, strings.Split(in.srcSpecPath, ".")...) - if !found { - return errors.Errorf("missing field %q in %s", in.srcSpecPath, tlog.KObj{Obj: in.src}) - } else if err != nil { - return errors.Wrapf(err, "failed to get field %q from %s", in.srcSpecPath, tlog.KObj{Obj: in.src}) - } - - // Set spec in dest. - if err := unstructured.SetNestedField(in.dest.Object, srcSpec, strings.Split(in.destSpecPath, ".")...); err != nil { - return errors.Wrapf(err, "failed to set field %q on %s", in.destSpecPath, tlog.KObj{Obj: in.dest}) - } - - // Restore preserved fields. - for path, value := range preservedFields { - if err := unstructured.SetNestedField(in.dest.Object, value, strings.Split(path, ".")...); err != nil { - return errors.Wrapf(err, "failed to set field %q on %s", path, tlog.KObj{Obj: in.dest}) - } - } - return nil -} diff --git a/controllers/topology/internal/extensions/patches/engine_test.go b/controllers/topology/internal/extensions/patches/engine_test.go new file mode 100644 index 000000000000..17637d7ec031 --- /dev/null +++ b/controllers/topology/internal/extensions/patches/engine_test.go @@ -0,0 +1,542 @@ +/* +Copyright 2021 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 patches + +import ( + "context" + "fmt" + "strings" + "testing" + + . "github.com/onsi/gomega" + "github.com/pkg/errors" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/utils/pointer" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/controllers/topology/internal/extensions/patches/api" + "sigs.k8s.io/cluster-api/controllers/topology/internal/scope" + "sigs.k8s.io/cluster-api/internal/builder" + . "sigs.k8s.io/cluster-api/internal/matchers" +) + +func TestApply(t *testing.T) { + type patch struct { + name string + patches []api.GenerateResponsePatch + } + type expectedFields struct { + infrastructureCluster map[string]interface{} + controlPlane map[string]interface{} + controlPlaneInfrastructureMachineTemplate map[string]interface{} + machineDeploymentBootstrapTemplate map[string]map[string]interface{} + machineDeploymentInfrastructureMachineTemplate map[string]map[string]interface{} + } + + tests := []struct { + name string + patches []patch + expectedFields expectedFields + }{ + { + name: "Should preserve desired state, if there are no patches", + }, + { + name: "Should apply JSON patches to InfraCluster, ControlPlane and ControlPlaneInfrastructureMachineTemplate", + patches: []patch{ + { + name: "fake-patch1", + patches: []api.GenerateResponsePatch{ + { + TemplateID: api.TemplateID{ + APIVersion: builder.InfrastructureGroupVersion.String(), + Kind: builder.GenericInfrastructureClusterTemplateKind, + TargetType: api.InfrastructureClusterTemplateTargetType, + }, + Patch: apiextensionsv1.JSON{Raw: []byte(` +[{"op":"add","path":"/spec/template/spec/resource","value":"infraCluster"}] + `)}, + PatchType: api.JSONPatchType, + }, + { + TemplateID: api.TemplateID{ + APIVersion: builder.ControlPlaneGroupVersion.String(), + Kind: builder.GenericControlPlaneTemplateKind, + TargetType: api.ControlPlaneTemplateTargetType, + }, + Patch: apiextensionsv1.JSON{Raw: []byte(` +[{"op":"add","path":"/spec/template/spec/resource","value":"controlPlane"}] + `)}, + PatchType: api.JSONPatchType, + }, + { + TemplateID: api.TemplateID{ + APIVersion: builder.InfrastructureGroupVersion.String(), + Kind: builder.GenericInfrastructureMachineTemplateKind, + TargetType: api.ControlPlaneInfrastructureMachineTemplateTargetType, + }, + Patch: apiextensionsv1.JSON{Raw: []byte(` +[{"op":"add","path":"/spec/template/spec/resource","value":"controlPlaneInfrastructureMachineTemplate"}] + `)}, + PatchType: api.JSONPatchType, + }, + }, + }}, + expectedFields: expectedFields{ + infrastructureCluster: map[string]interface{}{ + "spec.resource": "infraCluster", + }, + controlPlane: map[string]interface{}{ + "spec.resource": "controlPlane", + }, + controlPlaneInfrastructureMachineTemplate: map[string]interface{}{ + "spec.template.spec.resource": "controlPlaneInfrastructureMachineTemplate", + }, + }, + }, + { + name: "Should apply JSON patches to MachineDeployment templates", + patches: []patch{ + { + name: "fake-patch1", + patches: []api.GenerateResponsePatch{ + { + TemplateID: api.TemplateID{ + APIVersion: builder.InfrastructureGroupVersion.String(), + Kind: builder.GenericInfrastructureMachineTemplateKind, + TargetType: api.MachineDeploymentInfrastructureMachineTemplateTargetType, + MachineDeployment: api.MachineDeploymentID{ + TopologyName: "default-worker-topo1", + Class: "default-worker", + }, + }, + Patch: apiextensionsv1.JSON{Raw: []byte(` +[{"op":"add","path":"/spec/template/spec/resource","value":"topo1-infra"}] + `)}, + PatchType: api.JSONPatchType, + }, + { + TemplateID: api.TemplateID{ + APIVersion: builder.BootstrapGroupVersion.String(), + Kind: builder.GenericBootstrapConfigTemplateKind, + TargetType: api.MachineDeploymentBootstrapConfigTemplateTargetType, + MachineDeployment: api.MachineDeploymentID{ + TopologyName: "default-worker-topo1", + Class: "default-worker", + }, + }, + Patch: apiextensionsv1.JSON{Raw: []byte(` +[{"op":"add","path":"/spec/template/spec/resource","value":"topo1-bootstrap"}] + `)}, + PatchType: api.JSONPatchType, + }, + { + TemplateID: api.TemplateID{ + APIVersion: builder.InfrastructureGroupVersion.String(), + Kind: builder.GenericInfrastructureMachineTemplateKind, + TargetType: api.MachineDeploymentInfrastructureMachineTemplateTargetType, + MachineDeployment: api.MachineDeploymentID{ + TopologyName: "default-worker-topo2", + Class: "default-worker", + }, + }, + Patch: apiextensionsv1.JSON{Raw: []byte(` +[{"op":"add","path":"/spec/template/spec/resource","value":"topo2-infra"}] + `)}, + PatchType: api.JSONPatchType, + }, + { + TemplateID: api.TemplateID{ + APIVersion: builder.BootstrapGroupVersion.String(), + Kind: builder.GenericBootstrapConfigTemplateKind, + TargetType: api.MachineDeploymentBootstrapConfigTemplateTargetType, + MachineDeployment: api.MachineDeploymentID{ + TopologyName: "default-worker-topo2", + Class: "default-worker", + }, + }, + Patch: apiextensionsv1.JSON{Raw: []byte(` +[{"op":"add","path":"/spec/template/spec/resource","value":"topo2-bootstrap"}] + `)}, + PatchType: api.JSONPatchType, + }, + }, + }, + }, + expectedFields: expectedFields{ + machineDeploymentBootstrapTemplate: map[string]map[string]interface{}{ + "default-worker-topo1": {"spec.template.spec.resource": "topo1-bootstrap"}, + "default-worker-topo2": {"spec.template.spec.resource": "topo2-bootstrap"}, + }, + machineDeploymentInfrastructureMachineTemplate: map[string]map[string]interface{}{ + "default-worker-topo1": {"spec.template.spec.resource": "topo1-infra"}, + "default-worker-topo2": {"spec.template.spec.resource": "topo2-infra"}, + }, + }, + }, + { + name: "Should apply JSON patches in the correct order", + patches: []patch{ + { + name: "fake-patch1", + patches: []api.GenerateResponsePatch{ + { + TemplateID: api.TemplateID{ + APIVersion: builder.ControlPlaneGroupVersion.String(), + Kind: builder.GenericControlPlaneTemplateKind, + TargetType: api.ControlPlaneTemplateTargetType, + }, + Patch: apiextensionsv1.JSON{Raw: []byte(` +[{"op":"add","path":"/spec/template/spec/clusterName","value":"cluster1"}, +{"op":"add","path":"/spec/template/spec/files","value":[{"key1":"value1"}]}] + `)}, + PatchType: api.JSONPatchType, + }, + }, + }, + { + name: "fake-patch2", + patches: []api.GenerateResponsePatch{ + { + TemplateID: api.TemplateID{ + APIVersion: builder.ControlPlaneGroupVersion.String(), + Kind: builder.GenericControlPlaneTemplateKind, + TargetType: api.ControlPlaneTemplateTargetType, + }, + Patch: apiextensionsv1.JSON{Raw: []byte(` +[{"op":"replace","path":"/spec/template/spec/clusterName","value":"cluster1-overwritten"}] +`)}, + PatchType: api.JSONPatchType, + }, + }, + }, + }, + expectedFields: expectedFields{ + controlPlane: map[string]interface{}{ + "spec.clusterName": "cluster1-overwritten", + "spec.files": []interface{}{ + map[string]interface{}{ + "key1": "value1", + }, + }, + }, + }, + }, + { + name: "Should apply JSON patches and preserve ControlPlane fields", + patches: []patch{ + { + name: "fake-patch1", + patches: []api.GenerateResponsePatch{ + { + TemplateID: api.TemplateID{ + APIVersion: builder.ControlPlaneGroupVersion.String(), + Kind: builder.GenericControlPlaneTemplateKind, + TargetType: api.ControlPlaneTemplateTargetType, + }, + Patch: apiextensionsv1.JSON{Raw: []byte(` +[{"op":"replace","path":"/spec/template/spec/replicas","value":1}, +{"op":"replace","path":"/spec/template/spec/version","value":"v1.15.0"}, +{"op":"replace","path":"/spec/template/spec/machineTemplate/infrastructureRef","value":{"apiVersion":"invalid","kind":"invalid","namespace":"invalid","name":"invalid"}}] + `)}, + PatchType: api.JSONPatchType, + }, + }, + }, + }, + }, + { + name: "Should apply JSON patches without metadata", + patches: []patch{ + { + name: "fake-patch1", + patches: []api.GenerateResponsePatch{ + { + TemplateID: api.TemplateID{ + APIVersion: builder.InfrastructureGroupVersion.String(), + Kind: builder.GenericInfrastructureClusterTemplateKind, + TargetType: api.InfrastructureClusterTemplateTargetType, + }, + Patch: apiextensionsv1.JSON{Raw: []byte(` +[{"op":"add","path":"/spec/template/spec/clusterName","value":"cluster1"}, +{"op":"replace","path":"/metadata/name","value": "overwrittenName"}] + `)}, + PatchType: api.JSONPatchType, + }, + }, + }, + }, + expectedFields: expectedFields{ + infrastructureCluster: map[string]interface{}{ + "spec.clusterName": "cluster1", + }, + }, + }, + { + name: "Should apply JSON merge patches", + patches: []patch{ + { + name: "fake-patch1", + patches: []api.GenerateResponsePatch{ + { + TemplateID: api.TemplateID{ + APIVersion: builder.InfrastructureGroupVersion.String(), + Kind: builder.GenericInfrastructureClusterTemplateKind, + TargetType: api.InfrastructureClusterTemplateTargetType, + }, + Patch: apiextensionsv1.JSON{Raw: []byte(` +{"spec": {"resource": "infraCluster"}} + `)}, + PatchType: api.JSONMergePatchType, + }, + }, + }, + }, + expectedFields: expectedFields{ + infrastructureCluster: map[string]interface{}{ + "spec.resource": "infraCluster", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + // Set up test objects. + blueprint, desired := setupTestObjects() + + // If there are patches, set up patch generators. + if len(tt.patches) > 0 { + for _, patch := range tt.patches { + // Add the patches to ensure the patch generator is called. + blueprint.ClusterClass.Spec.Patches = append(blueprint.ClusterClass.Spec.Patches, clusterv1.ClusterClassPatch{Name: patch.name}) + } + + // Overwrite createPatchGenerator temporarily, so that it returns the + // fakePatchGenerator corresponding to the current patch. + createPatchGeneratorOrig := createPatchGenerator + defer func() { + createPatchGenerator = createPatchGeneratorOrig + }() + createPatchGenerator = func(patch *clusterv1.ClusterClassPatch) (api.Generator, error) { + for _, p := range tt.patches { + if p.name == patch.Name { + return &fakePatchGenerator{patches: p.patches}, nil + } + } + return nil, errors.Errorf("could not find patch generator for patch %q", patch.Name) + } + } + + // Copy the desired objects before applying patches. + expectedCluster := desired.Cluster.DeepCopy() + expectedInfrastructureCluster := desired.InfrastructureCluster.DeepCopy() + expectedControlPlane := desired.ControlPlane.Object.DeepCopy() + expectedControlPlaneInfrastructureMachineTemplate := desired.ControlPlane.InfrastructureMachineTemplate.DeepCopy() + expectedBootstrapTemplates := map[string]*unstructured.Unstructured{} + expectedInfrastructureMachineTemplate := map[string]*unstructured.Unstructured{} + for mdTopology, md := range desired.MachineDeployments { + expectedBootstrapTemplates[mdTopology] = md.BootstrapTemplate.DeepCopy() + expectedInfrastructureMachineTemplate[mdTopology] = md.InfrastructureMachineTemplate.DeepCopy() + } + // Set expected fields. + if tt.expectedFields.infrastructureCluster != nil { + setSpecFields(expectedInfrastructureCluster, tt.expectedFields.infrastructureCluster) + } + if tt.expectedFields.controlPlane != nil { + setSpecFields(expectedControlPlane, tt.expectedFields.controlPlane) + } + if tt.expectedFields.controlPlaneInfrastructureMachineTemplate != nil { + setSpecFields(expectedControlPlaneInfrastructureMachineTemplate, tt.expectedFields.controlPlaneInfrastructureMachineTemplate) + } + for mdTopology, expectedFields := range tt.expectedFields.machineDeploymentBootstrapTemplate { + setSpecFields(expectedBootstrapTemplates[mdTopology], expectedFields) + } + for mdTopology, expectedFields := range tt.expectedFields.machineDeploymentInfrastructureMachineTemplate { + setSpecFields(expectedInfrastructureMachineTemplate[mdTopology], expectedFields) + } + + // Apply patches. + g.Expect(Apply(context.Background(), blueprint, desired)).To(Succeed()) + + // Compare the patched desired objects with the expected desired objects. + g.Expect(desired.Cluster).To(EqualObject(expectedCluster, IgnoreAutogeneratedMetadata)) + g.Expect(desired.InfrastructureCluster).To(EqualObject(expectedInfrastructureCluster)) + g.Expect(desired.ControlPlane.Object).To(EqualObject(expectedControlPlane)) + g.Expect(desired.ControlPlane.InfrastructureMachineTemplate).To(EqualObject(expectedControlPlaneInfrastructureMachineTemplate)) + for mdTopology, bootstrapTemplate := range expectedBootstrapTemplates { + g.Expect(desired.MachineDeployments[mdTopology].BootstrapTemplate).To(EqualObject(bootstrapTemplate)) + } + for mdTopology, infrastructureMachineTemplate := range expectedInfrastructureMachineTemplate { + g.Expect(desired.MachineDeployments[mdTopology].InfrastructureMachineTemplate).To(EqualObject(infrastructureMachineTemplate)) + } + }) + } +} + +func setupTestObjects() (*scope.ClusterBlueprint, *scope.ClusterState) { + infrastructureClusterTemplate := builder.InfrastructureClusterTemplate(metav1.NamespaceDefault, "infraClusterTemplate1"). + Build() + + controlPlaneInfrastructureMachineTemplate := builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "controlplaneinframachinetemplate1"). + Build() + controlPlaneTemplate := builder.ControlPlaneTemplate(metav1.NamespaceDefault, "controlPlaneTemplate1"). + WithInfrastructureMachineTemplate(controlPlaneInfrastructureMachineTemplate). + Build() + + workerInfrastructureMachineTemplate := builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "linux-worker-inframachinetemplate"). + Build() + workerBootstrapTemplate := builder.BootstrapTemplate(metav1.NamespaceDefault, "linux-worker-bootstraptemplate"). + Build() + mdClass1 := builder.MachineDeploymentClass(metav1.NamespaceDefault, "class1"). + WithClass("default-worker"). + WithInfrastructureTemplate(workerInfrastructureMachineTemplate). + WithBootstrapTemplate(workerBootstrapTemplate). + Build() + + clusterClass := builder.ClusterClass(metav1.NamespaceDefault, "clusterClass1"). + WithInfrastructureClusterTemplate(infrastructureClusterTemplate). + WithControlPlaneTemplate(controlPlaneTemplate). + WithControlPlaneInfrastructureMachineTemplate(controlPlaneInfrastructureMachineTemplate). + WithWorkerMachineDeploymentClasses([]clusterv1.MachineDeploymentClass{*mdClass1}). + Build() + + cluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: metav1.NamespaceDefault, + }, + Spec: clusterv1.ClusterSpec{ + Topology: &clusterv1.Topology{ + Version: "v1.21.2", + Class: clusterClass.Name, + ControlPlane: clusterv1.ControlPlaneTopology{ + Replicas: pointer.Int32(3), + }, + Workers: &clusterv1.WorkersTopology{ + MachineDeployments: []clusterv1.MachineDeploymentTopology{ + { + Metadata: clusterv1.ObjectMeta{}, + Class: "default-worker", + Name: "default-worker-topo1", + }, + { + Metadata: clusterv1.ObjectMeta{}, + Class: "default-worker", + Name: "default-worker-topo2", + Replicas: pointer.Int32(5), + }, + }, + }, + }, + }, + } + + // Aggregating Cluster, Templates and ClusterClass into a blueprint. + blueprint := &scope.ClusterBlueprint{ + Topology: cluster.Spec.Topology, + ClusterClass: clusterClass, + InfrastructureClusterTemplate: infrastructureClusterTemplate, + ControlPlane: &scope.ControlPlaneBlueprint{ + Template: controlPlaneTemplate, + InfrastructureMachineTemplate: controlPlaneInfrastructureMachineTemplate, + }, + MachineDeployments: map[string]*scope.MachineDeploymentBlueprint{ + "default-worker": { + InfrastructureMachineTemplate: workerInfrastructureMachineTemplate, + BootstrapTemplate: workerBootstrapTemplate, + }, + }, + } + + // Create a Cluster using the ClusterClass from above with multiple MachineDeployments + // using the same MachineDeployment class. + desiredCluster := cluster.DeepCopy() + + infrastructureCluster := builder.InfrastructureCluster(metav1.NamespaceDefault, "infraClusterTemplate1"). + WithSpecFields(map[string]interface{}{ + // Add an empty spec field, to make sure the InfrastructureCluster matches + // the one calculated by computeInfrastructureCluster. + "spec": map[string]interface{}{}, + }). + Build() + + controlPlane := builder.ControlPlane(metav1.NamespaceDefault, "controlPlane1"). + WithVersion("v1.21.2"). + WithReplicas(3). + // Make sure we're using an independent instance of the template. + WithInfrastructureMachineTemplate(controlPlaneInfrastructureMachineTemplate.DeepCopy()). + Build() + + desired := &scope.ClusterState{ + Cluster: desiredCluster, + InfrastructureCluster: infrastructureCluster, + ControlPlane: &scope.ControlPlaneState{ + Object: controlPlane, + // Make sure we're using an independent instance of the template. + InfrastructureMachineTemplate: controlPlaneInfrastructureMachineTemplate.DeepCopy(), + }, + MachineDeployments: map[string]*scope.MachineDeploymentState{ + "default-worker-topo1": { + Object: builder.MachineDeployment(metav1.NamespaceDefault, "md1"). + WithVersion("v1.21.2"). + Build(), + // Make sure we're using an independent instance of the template. + InfrastructureMachineTemplate: workerInfrastructureMachineTemplate.DeepCopy(), + BootstrapTemplate: workerBootstrapTemplate.DeepCopy(), + }, + "default-worker-topo2": { + Object: builder.MachineDeployment(metav1.NamespaceDefault, "md2"). + WithVersion("v1.20.6"). + WithReplicas(5). + Build(), + // Make sure we're using an independent instance of the template. + InfrastructureMachineTemplate: workerInfrastructureMachineTemplate.DeepCopy(), + BootstrapTemplate: workerBootstrapTemplate.DeepCopy(), + }, + }, + } + return blueprint, desired +} + +type fakePatchGenerator struct { + patches []api.GenerateResponsePatch +} + +func (g *fakePatchGenerator) Generate(_ context.Context, _ *api.GenerateRequest) (*api.GenerateResponse, error) { + return &api.GenerateResponse{ + Items: g.patches, + }, nil +} + +// setSpecFields sets fields on an unstructured object from a map. +func setSpecFields(obj *unstructured.Unstructured, fields map[string]interface{}) { + for k, v := range fields { + fieldParts := strings.Split(k, ".") + if len(fieldParts) == 0 { + panic(fmt.Errorf("fieldParts invalid")) + } + if fieldParts[0] != "spec" { + panic(fmt.Errorf("can not set fields outside spec")) + } + if err := unstructured.SetNestedField(obj.UnstructuredContent(), v, strings.Split(k, ".")...); err != nil { + panic(err) + } + } +} diff --git a/controllers/topology/internal/extensions/patches/patch.go b/controllers/topology/internal/extensions/patches/patch.go new file mode 100644 index 000000000000..439f6a2ebd31 --- /dev/null +++ b/controllers/topology/internal/extensions/patches/patch.go @@ -0,0 +1,182 @@ +package patches + +import ( + "context" + "strings" + + "github.com/pkg/errors" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/cluster-api/controllers/topology/internal/contract" + tlog "sigs.k8s.io/cluster-api/controllers/topology/internal/log" + "sigs.k8s.io/cluster-api/controllers/topology/internal/mergepatch" +) + +// PatchOption represents an option for the patchObject and patchTemplate funcs. +type PatchOption interface { + // ApplyToHelper applies configuration to the given options. + ApplyToHelper(*PatchOptions) +} + +// PatchOptions contains options for patchObject and patchTemplate. +type PatchOptions struct { + preserveFields []contract.Path +} + +// ApplyOptions applies the given patch options +// and then returns itself (for convenient chaining). +func (o *PatchOptions) ApplyOptions(opts []PatchOption) *PatchOptions { + for _, opt := range opts { + opt.ApplyToHelper(o) + } + return o +} + +// PreserveFields instructs the patch func to preserve fields. +type PreserveFields []contract.Path + +// ApplyToHelper applies this configuration to the given patch options. +func (i PreserveFields) ApplyToHelper(opts *PatchOptions) { + opts.preserveFields = i +} + +// patchObject overwrites spec in object with spec.template.spec of patchedTemplate, +// while preserving the configured fields. +func patchObject(ctx context.Context, object, patchedTemplate *unstructured.Unstructured, opts ...PatchOption) error { + log := tlog.LoggerFrom(ctx) + + patchOptions := &PatchOptions{} + patchOptions = patchOptions.ApplyOptions(opts) + + // Create a copy of the object. + original := object.DeepCopy() + + if err := copySpec(copySpecInput{ + src: patchedTemplate, + dest: object, + srcSpecPath: "spec.template.spec", + destSpecPath: "spec", + fieldsToPreserve: patchOptions.preserveFields, + }); err != nil { + return errors.Wrapf(err, "failed to patch object %s by appying changes from the patched template", + tlog.KObj{Obj: object}) + } + + // Log the delta between the object before and after applying the accumulated patches. + helper, err := mergepatch.NewHelper(original, object, nil) + if err != nil { + return err + } + log.V(4).WithObject(object).Infof("Applying accumulated patches", "delta", string(helper.Changes())) + + return nil +} + +// patchTemplate overwrites spec.template.spec in template with spec.template.spec of patchedTemplate, +// while preserving the configured fields. +func patchTemplate(ctx context.Context, template, patchedTemplate *unstructured.Unstructured, opts ...PatchOption) error { + log := tlog.LoggerFrom(ctx) + + patchOptions := &PatchOptions{} + patchOptions = patchOptions.ApplyOptions(opts) + + // Create a copy of the template. + original := template.DeepCopy() + + if err := copySpec(copySpecInput{ + src: patchedTemplate, + dest: template, + srcSpecPath: "spec.template.spec", + destSpecPath: "spec.template.spec", + fieldsToPreserve: patchOptions.preserveFields, + }); err != nil { + return errors.Wrapf(err, "failed to patch template %s by appying changes from the patched template", + tlog.KObj{Obj: template}) + } + + // Log the delta between the object before and after applying the accumulated patches. + helper, err := mergepatch.NewHelper(original, template, nil) + if err != nil { + return err + } + log.V(4).WithObject(template).Infof("Applying accumulated patches", "delta", string(helper.Changes())) + + return nil +} + +// patchTemplateSpec overwrites spec in templateJSON with spec of patchedTemplateBytes. +func patchTemplateSpec(templateJSON *apiextensionsv1.JSON, patchedTemplateBytes []byte) error { + // Convert templates to Unstructured. + template, err := bytesToUnstructured(templateJSON.Raw) + if err != nil { + return errors.Wrap(err, "failed to convert template to Unstructured") + } + patchedTemplate, err := bytesToUnstructured(patchedTemplateBytes) + if err != nil { + return errors.Wrap(err, "failed to convert patched template to Unstructured") + } + + // Copy spec from patchedTemplate to template. + if err := copySpec(copySpecInput{ + src: patchedTemplate, + dest: template, + srcSpecPath: "spec", + destSpecPath: "spec", + }); err != nil { + return errors.Wrap(err, "failed to apply patch to template") + } + + // Marshal template and store it in templateJSON. + templateBytes, err := template.MarshalJSON() + if err != nil { + return errors.Wrapf(err, "failed to marshal patched template") + } + templateJSON.Raw = templateBytes + return nil +} + +type copySpecInput struct { + src *unstructured.Unstructured + dest *unstructured.Unstructured + srcSpecPath string + destSpecPath string + fieldsToPreserve []contract.Path +} + +// copySpec copies a field from a srcSpecPath in src to a destSpecPath in dest, +// while preserving fieldsToPreserve. +func copySpec(in copySpecInput) error { + // Backup fields that should be preserved from dest. + preservedFields := map[string]interface{}{} + for _, field := range in.fieldsToPreserve { + value, found, err := unstructured.NestedFieldNoCopy(in.dest.Object, field...) + if !found { + // Continue if the field does not exist in src. fieldsToPreserve don't have to exist. + continue + } else if err != nil { + return errors.Wrapf(err, "failed to get field %q from %s", strings.Join(field, "."), tlog.KObj{Obj: in.dest}) + } + preservedFields[strings.Join(field, ".")] = value + } + + // Get spec from src. + srcSpec, found, err := unstructured.NestedFieldNoCopy(in.src.Object, strings.Split(in.srcSpecPath, ".")...) + if !found { + return errors.Errorf("missing field %q in %s", in.srcSpecPath, tlog.KObj{Obj: in.src}) + } else if err != nil { + return errors.Wrapf(err, "failed to get field %q from %s", in.srcSpecPath, tlog.KObj{Obj: in.src}) + } + + // Set spec in dest. + if err := unstructured.SetNestedField(in.dest.Object, srcSpec, strings.Split(in.destSpecPath, ".")...); err != nil { + return errors.Wrapf(err, "failed to set field %q on %s", in.destSpecPath, tlog.KObj{Obj: in.dest}) + } + + // Restore preserved fields. + for path, value := range preservedFields { + if err := unstructured.SetNestedField(in.dest.Object, value, strings.Split(path, ".")...); err != nil { + return errors.Wrapf(err, "failed to set field %q on %s", path, tlog.KObj{Obj: in.dest}) + } + } + return nil +} diff --git a/controllers/topology/internal/extensions/patches/patch_test.go b/controllers/topology/internal/extensions/patches/patch_test.go new file mode 100644 index 000000000000..2e1b41b30e2c --- /dev/null +++ b/controllers/topology/internal/extensions/patches/patch_test.go @@ -0,0 +1,369 @@ +/* +Copyright 2021 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 patches + +import ( + "testing" + + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/cluster-api/controllers/topology/internal/contract" +) + +func TestCopySpec(t *testing.T) { + tests := []struct { + name string + input copySpecInput + want *unstructured.Unstructured + wantErr bool + }{ + { + name: "Field both in src and dest, no-op when equal", + input: copySpecInput{ + src: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + dest: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + srcSpecPath: "spec", + destSpecPath: "spec", + }, + want: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + }, + { + name: "Field both in src and dest, overwrite dest when different", + input: copySpecInput{ + src: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + dest: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A-different", + }, + }, + }, + srcSpecPath: "spec", + destSpecPath: "spec", + }, + want: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + }, + { + name: "Nested field both in src and dest, no-op when equal", + input: copySpecInput{ + src: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + }, + }, + dest: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + }, + }, + srcSpecPath: "spec", + destSpecPath: "spec", + }, + want: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + }, + }, + }, + { + name: "Nested field both in src and dest, overwrite dest when different", + input: copySpecInput{ + src: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + }, + }, + dest: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A-different", + }, + }, + }, + }, + }, + srcSpecPath: "spec", + destSpecPath: "spec", + }, + want: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + }, + }, + }, + { + name: "Field only in src, copy to dest", + input: copySpecInput{ + src: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "foo": "bar", + }, + }, + }, + dest: &unstructured.Unstructured{ + Object: map[string]interface{}{}, + }, + srcSpecPath: "spec", + destSpecPath: "spec", + }, + want: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "foo": "bar", + }, + }, + }, + }, + { + name: "Nested field only in src, copy to dest", + input: copySpecInput{ + src: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + }, + }, + dest: &unstructured.Unstructured{ + Object: map[string]interface{}{}, + }, + srcSpecPath: "spec", + destSpecPath: "spec", + }, + want: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + }, + }, + }, + { + name: "Copy field from spec.template.spec in src to spec in dest", + input: copySpecInput{ + src: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + }, + }, + dest: &unstructured.Unstructured{ + Object: map[string]interface{}{}, + }, + srcSpecPath: "spec.template.spec", + destSpecPath: "spec", + }, + want: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + }, + { + name: "Copy field from spec.template.spec in src to spec in dest (overwrite when different)", + input: copySpecInput{ + src: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + }, + }, + dest: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A-different", + }, + }, + }, + srcSpecPath: "spec.template.spec", + destSpecPath: "spec", + }, + want: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "A": "A", + }, + }, + }, + }, + { + name: "Field both in src and dest, overwrite when different and preserve fields", + input: copySpecInput{ + src: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "machineTemplate": map[string]interface{}{ + "infrastructureRef": map[string]interface{}{ + "apiVersion": "invalid", + "kind": "invalid", + "namespace": "invalid", + "name": "invalid", + }, + }, + "replicas": float64(10), + "version": "v1.15.0", + "A": "A", + }, + }, + }, + }, + }, + dest: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "machineTemplate": map[string]interface{}{ + "infrastructureRef": map[string]interface{}{ + "apiVersion": "v1", + "kind": "kind", + "namespace": "namespace", + "name": "name", + }, + }, + "replicas": float64(3), + "version": "v1.22.0", + "A": "A-different", + }, + }, + }, + srcSpecPath: "spec.template.spec", + destSpecPath: "spec", + fieldsToPreserve: []contract.Path{ + {"spec", "machineTemplate", "infrastructureRef"}, + {"spec", "replicas"}, + {"spec", "version"}, + }, + }, + want: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "machineTemplate": map[string]interface{}{ + "infrastructureRef": map[string]interface{}{ + "apiVersion": "v1", + "kind": "kind", + "namespace": "namespace", + "name": "name", + }, + }, + "replicas": float64(3), + "version": "v1.22.0", + "A": "A", + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + err := copySpec(tt.input) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(tt.input.dest).To(Equal(tt.want)) + }) + } +} diff --git a/controllers/topology/internal/extensions/patches/variables/variables_test.go b/controllers/topology/internal/extensions/patches/variables/variables_test.go new file mode 100644 index 000000000000..c589b984d0de --- /dev/null +++ b/controllers/topology/internal/extensions/patches/variables/variables_test.go @@ -0,0 +1,190 @@ +/* +Copyright 2021 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 variables + +import ( + "testing" + + . "github.com/onsi/gomega" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/utils/pointer" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/internal/builder" +) + +func TestGlobal(t *testing.T) { + tests := []struct { + name string + clusterTopology *clusterv1.Topology + cluster *clusterv1.Cluster + want VariableMap + }{ + { + name: "Should calculate global variables", + clusterTopology: &clusterv1.Topology{ + Variables: []clusterv1.ClusterVariable{ + { + Name: "location", + Value: toJSON("\"us-central\""), + }, + { + Name: "cpu", + Value: toJSON("8"), + }, + { + // This is blocked by a webhook, but let's make sure we overwrite + // the user-defined variable with the builtin variable anyway. + Name: "builtin.cluster.name", + Value: toJSON("8"), + }, + }, + }, + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: metav1.NamespaceDefault, + }, + Spec: clusterv1.ClusterSpec{ + Topology: &clusterv1.Topology{ + Class: "clusterClass1", + Version: "v1.21.1", + }, + }, + }, + want: VariableMap{ + "location": toJSON("\"us-central\""), + "cpu": toJSON("8"), + BuiltinClusterName: toJSON("\"cluster1\""), + BuiltinClusterNamespace: toJSON("\"default\""), + BuiltinClusterTopologyVersion: toJSON("\"v1.21.1\""), + BuiltinClusterTopologyClass: toJSON("\"clusterClass1\""), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got, err := Global(tt.clusterTopology, tt.cluster) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(tt.want)) + }) + } +} + +func TestControlPlane(t *testing.T) { + tests := []struct { + name string + controlPlaneTopology *clusterv1.ControlPlaneTopology + controlPlane *unstructured.Unstructured + want VariableMap + }{ + { + name: "Should calculate ControlPlane variables", + controlPlaneTopology: &clusterv1.ControlPlaneTopology{ + Replicas: pointer.Int32(3), + }, + controlPlane: builder.ControlPlane(metav1.NamespaceDefault, "controlPlane1"). + WithReplicas(3). + WithVersion("v1.21.1"). + Build(), + want: VariableMap{ + BuiltinClusterTopologyControlPlaneReplicas: toJSON("3"), + BuiltinClusterTopologyControlPlaneVersion: toJSON("\"v1.21.1\""), + }, + }, + { + name: "Should calculate ControlPlane variables, replicas not set", + controlPlaneTopology: &clusterv1.ControlPlaneTopology{}, + controlPlane: builder.ControlPlane(metav1.NamespaceDefault, "controlPlane1"). + WithVersion("v1.21.1"). + Build(), + want: VariableMap{ + BuiltinClusterTopologyControlPlaneVersion: toJSON("\"v1.21.1\""), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got, err := ControlPlane(tt.controlPlaneTopology, tt.controlPlane) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(tt.want)) + }) + } +} + +func TestMachineDeployment(t *testing.T) { + tests := []struct { + name string + mdTopology *clusterv1.MachineDeploymentTopology + md *clusterv1.MachineDeployment + want VariableMap + }{ + { + name: "Should calculate MachineDeployment variables", + mdTopology: &clusterv1.MachineDeploymentTopology{ + Replicas: pointer.Int32(3), + Name: "md-topology", + Class: "md-class", + }, + md: builder.MachineDeployment(metav1.NamespaceDefault, "md1"). + WithReplicas(3). + WithVersion("v1.21.1"). + Build(), + want: VariableMap{ + BuiltinClusterTopologyMachineDeploymentReplicas: toJSON("3"), + BuiltinClusterTopologyMachineDeploymentVersion: toJSON("\"v1.21.1\""), + BuiltinClusterTopologyMachineDeploymentClass: toJSON("\"md-class\""), + BuiltinClusterTopologyMachineDeploymentName: toJSON("\"md1\""), + BuiltinClusterTopologyMachineDeploymentTopologyName: toJSON("\"md-topology\""), + }, + }, + { + name: "Should calculate MachineDeployment variables, replicas not set", + mdTopology: &clusterv1.MachineDeploymentTopology{ + Name: "md-topology", + Class: "md-class", + }, + md: builder.MachineDeployment(metav1.NamespaceDefault, "md1"). + WithVersion("v1.21.1"). + Build(), + want: VariableMap{ + BuiltinClusterTopologyMachineDeploymentVersion: toJSON("\"v1.21.1\""), + BuiltinClusterTopologyMachineDeploymentClass: toJSON("\"md-class\""), + BuiltinClusterTopologyMachineDeploymentName: toJSON("\"md1\""), + BuiltinClusterTopologyMachineDeploymentTopologyName: toJSON("\"md-topology\""), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got, err := MachineDeployment(tt.mdTopology, tt.md) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(tt.want)) + }) + } +} + +func toJSON(value string) apiextensionsv1.JSON { + return apiextensionsv1.JSON{Raw: []byte(value)} +} diff --git a/internal/builder/builders.go b/internal/builder/builders.go index 322936e5bac4..2016319c40b7 100644 --- a/internal/builder/builders.go +++ b/internal/builder/builders.go @@ -554,6 +554,8 @@ type ControlPlaneBuilder struct { namespace string name string infrastructureMachineTemplate *unstructured.Unstructured + replicas *int64 + version *string specFields map[string]interface{} statusFields map[string]interface{} } @@ -572,6 +574,18 @@ func (f *ControlPlaneBuilder) WithInfrastructureMachineTemplate(t *unstructured. return f } +// WithReplicas sets the number of replicas for the ControlPlaneBuilder. +func (f *ControlPlaneBuilder) WithReplicas(replicas int64) *ControlPlaneBuilder { + f.replicas = &replicas + return f +} + +// WithVersion adds the passed version to the ControlPlaneBuilder. +func (f *ControlPlaneBuilder) WithVersion(version string) *ControlPlaneBuilder { + f.version = &version + return f +} + // WithSpecFields sets a map of spec fields on the unstructured object. The keys in the map represent the path and the value corresponds // to the value of the spec field. // @@ -609,12 +623,23 @@ func (f *ControlPlaneBuilder) Build() *unstructured.Unstructured { setSpecFields(obj, f.specFields) setStatusFields(obj, f.statusFields) + // TODO(killianmuldoon): Update to use the internal/contract package, when it is importable from here if f.infrastructureMachineTemplate != nil { - // TODO(killianmuldoon): Update to use the internal/contract package if err := setNestedRef(obj, f.infrastructureMachineTemplate, "spec", "machineTemplate", "infrastructureRef"); err != nil { panic(err) } } + if f.replicas != nil { + if err := unstructured.SetNestedField(obj.UnstructuredContent(), *f.replicas, "spec", "replicas"); err != nil { + panic(err) + } + } + if f.version != nil { + if err := unstructured.SetNestedField(obj.UnstructuredContent(), *f.version, "spec", "version"); err != nil { + panic(err) + } + } + return obj }