diff --git a/internal/controllers/topology/cluster/patches/api/interface.go b/internal/controllers/topology/cluster/patches/api/interface.go index 8a50a5e336f0..5fd5aef3b091 100644 --- a/internal/controllers/topology/cluster/patches/api/interface.go +++ b/internal/controllers/topology/cluster/patches/api/interface.go @@ -23,128 +23,14 @@ package api import ( "context" - "fmt" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" ) // Generator defines a component that can generate patches for ClusterClass templates. type Generator interface { // Generate generates patches for templates. - // GenerateRequest contains templates and the corresponding variables. - // GenerateResponse contains the patches which should be applied to the templates of the GenerateRequest. - Generate(ctx context.Context, request *GenerateRequest) (*GenerateResponse, error) -} - -// GenerateRequest defines the input for a Generate request. -type GenerateRequest struct { - // Variables is a name/value map containing variables. - Variables map[string]apiextensionsv1.JSON - - // Items contains the list of templates to generate patches for. - Items []*GenerateRequestTemplate -} - -// GenerateRequestTemplate defines one of the ClusterClass templates to generate patches for. -type GenerateRequestTemplate struct { - // TemplateRef identifies a template to generate patches for; - // the same TemplateRef must be used when specifying to which template a generated patch should be applied to. - TemplateRef TemplateRef - - // Variables is a name/value map containing variables specifically for the current template. - // For example some builtin variables like MachineDeployment replicas and version are context-sensitive - // and thus are only added to templates for MachineDeployments and with values which correspond to the - // current MachineDeployment. - Variables map[string]apiextensionsv1.JSON - - // Template contains the template. - Template apiextensionsv1.JSON -} - -// TemplateRef identifies one of the ClusterClass templates to generate patches for; -// the same TemplateRef must be used when specifying where a generated patch should apply to. -type TemplateRef struct { - // APIVersion of the current template. - APIVersion string - - // Kind of the current template. - Kind string - - // TemplateType defines where the template is used. - TemplateType TemplateType - - // MachineDeployment specifies the MachineDeployment in which the template is used. - // This field is only set if the template is used in the context of a MachineDeployment. - MachineDeploymentRef MachineDeploymentRef -} - -func (t TemplateRef) String() string { - ret := fmt.Sprintf("%s %s/%s", t.TemplateType, t.APIVersion, t.Kind) - if t.MachineDeploymentRef.TopologyName != "" { - ret = fmt.Sprintf("%s, MachineDeployment topology %s", ret, t.MachineDeploymentRef.TopologyName) - } - if t.MachineDeploymentRef.Class != "" { - ret = fmt.Sprintf("%s, MachineDeployment class %s", ret, t.MachineDeploymentRef.Class) - } - return ret -} - -// MachineDeploymentRef specifies the MachineDeployment in which the template is used. -type MachineDeploymentRef struct { - // TopologyName is the name of the MachineDeploymentTopology. - TopologyName string - - // Class is the name of the MachineDeploymentClass. - Class string -} - -// TemplateType define the type for target types enum. -type TemplateType string - -const ( - // InfrastructureClusterTemplateType identifies a template for the InfrastructureCluster object. - InfrastructureClusterTemplateType TemplateType = "InfrastructureClusterTemplate" - - // ControlPlaneTemplateType identifies a template for the ControlPlane object. - ControlPlaneTemplateType TemplateType = "ControlPlaneTemplate" - - // ControlPlaneInfrastructureMachineTemplateType identifies a template for the InfrastructureMachines to be used for the ControlPlane object. - ControlPlaneInfrastructureMachineTemplateType TemplateType = "ControlPlane/InfrastructureMachineTemplate" - - // MachineDeploymentBootstrapConfigTemplateType identifies a template for the BootstrapConfig to be used for a MachineDeployment object. - MachineDeploymentBootstrapConfigTemplateType TemplateType = "MachineDeployment/BootstrapConfigTemplate" - - // MachineDeploymentInfrastructureMachineTemplateType identifies a template for the InfrastructureMachines to be used for a MachineDeployment object. - MachineDeploymentInfrastructureMachineTemplateType TemplateType = "MachineDeployment/InfrastructureMachineTemplate" -) - -// PatchType define the type for patch types enum. -type PatchType string - -const ( - // JSONPatchType identifies a https://datatracker.ietf.org/doc/html/rfc6902 json patch. - JSONPatchType PatchType = "JSONPatch" - - // JSONMergePatchType identifies a https://datatracker.ietf.org/doc/html/rfc7386 json merge patch. - JSONMergePatchType PatchType = "JSONMergePatch" -) - -// GenerateResponse defines the response of a Generate request. -// NOTE: Patches defined in GenerateResponse will be applied in the same order to the original -// GenerateRequest object, thus adding changes on templates across all the subsequent Generate calls. -type GenerateResponse struct { - // Items contains the list of generated patches. - Items []GenerateResponsePatch -} - -// GenerateResponsePatch defines a Patch targeting a specific GenerateRequestTemplate. -type GenerateResponsePatch struct { - // TemplateRef identifies the template the patch should apply to. - TemplateRef TemplateRef - - // Patch contains the patch. - Patch apiextensionsv1.JSON - - // Patch defines the type of the JSON patch. - PatchType PatchType + // GeneratePatchesRequest contains templates and the corresponding variables. + // GeneratePatchesResponse contains the patches which should be applied to the templates of the GenerateRequest. + Generate(context.Context, *runtimehooksv1.GeneratePatchesRequest) *runtimehooksv1.GeneratePatchesResponse } diff --git a/internal/controllers/topology/cluster/patches/engine.go b/internal/controllers/topology/cluster/patches/engine.go index 227a6ce7ea43..b73011771f26 100644 --- a/internal/controllers/topology/cluster/patches/engine.go +++ b/internal/controllers/topology/cluster/patches/engine.go @@ -24,6 +24,7 @@ import ( "github.com/pkg/errors" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" "sigs.k8s.io/cluster-api/internal/contract" "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/api" "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/inline" @@ -54,9 +55,9 @@ type engine struct { // Apply applies patches to the desired state according to the patches from the ClusterClass, variables from the Cluster // and builtin variables. -// * A GenerateRequest with all templates and global and template-specific variables is created. +// * A GeneratePatchesRequest with all templates and global and template-specific variables is created. // * Then for all ClusterClassPatches of a ClusterClass, JSON or JSON merge patches are generated -// and successively applied to the templates in the GenerateRequest. +// and successively applied to the templates in the GeneratePatchesRequest. // * Eventually the patched templates are used to update the specs of the desired objects. func (e *engine) Apply(ctx context.Context, blueprint *scope.ClusterBlueprint, desired *scope.ClusterState) error { // Return if there are no patches. @@ -90,8 +91,8 @@ func (e *engine) Apply(ctx context.Context, blueprint *scope.ClusterBlueprint, d // NOTE: All the partial patches accumulate on top of the request, so the // patch generator in the next iteration of the loop will get the modified // version of the request (including the patched version of the templates). - resp, err := generator.Generate(ctx, req) - if err != nil { + resp := generator.Generate(ctx, req) + if resp.Status == "Failure" { return errors.Wrapf(err, "failed to generate patches for patch %q", clusterClassPatch.Name) } @@ -110,15 +111,15 @@ func (e *engine) Apply(ctx context.Context, blueprint *scope.ClusterBlueprint, d return nil } -// createRequest creates a GenerateRequest based on the ClusterBlueprint and the desired state. +// createRequest creates a GeneratePatchesRequest based on the ClusterBlueprint and the desired state. // ClusterBlueprint supplies the templates. Desired state is used to calculate variables which are later used // as input for the patch generation. // NOTE: GenerateRequestTemplates are created for the templates of each individual MachineDeployment in the desired // state. This is necessary because some builtin variables are MachineDeployment specific. For example version and // replicas of a MachineDeployment. -// NOTE: A single GenerateRequest object is used to carry templates state across subsequent Generate calls. -func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterState) (*api.GenerateRequest, error) { - req := &api.GenerateRequest{} +// NOTE: A single GeneratePatchesRequest object is used to carry templates state across subsequent Generate calls. +func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterState) (*runtimehooksv1.GeneratePatchesRequest, error) { + req := &runtimehooksv1.GeneratePatchesRequest{} // Calculate global variables. globalVariables, err := variables.Global(blueprint.Topology, desired.Cluster) @@ -129,13 +130,13 @@ func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterStat // Add the InfrastructureClusterTemplate. t, err := newTemplateBuilder(blueprint.InfrastructureClusterTemplate). - WithType(api.InfrastructureClusterTemplateType). + WithHolder(desired.Cluster, "spec.infrastructureRef"). Build() if err != nil { return nil, errors.Wrapf(err, "failed to prepare InfrastructureCluster template %s for patching", tlog.KObj{Obj: blueprint.InfrastructureClusterTemplate}) } - req.Items = append(req.Items, t) + req.Items = append(req.Items, *t) // Calculate controlPlane variables. controlPlaneVariables, err := variables.ControlPlane(&blueprint.Topology.ControlPlane, desired.ControlPlane.Object, desired.ControlPlane.InfrastructureMachineTemplate) @@ -145,27 +146,27 @@ func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterStat // Add the ControlPlaneTemplate. t, err = newTemplateBuilder(blueprint.ControlPlane.Template). - WithType(api.ControlPlaneTemplateType). + WithHolder(desired.Cluster, "spec.controlPlaneRef"). Build() if err != nil { return nil, errors.Wrapf(err, "failed to prepare ControlPlane template %s for patching", tlog.KObj{Obj: blueprint.ControlPlane.Template}) } t.Variables = controlPlaneVariables - req.Items = append(req.Items, t) + req.Items = append(req.Items, *t) // If the clusterClass mandates the controlPlane has infrastructureMachines, // add the InfrastructureMachineTemplate for control plane machines. if blueprint.HasControlPlaneInfrastructureMachine() { t, err := newTemplateBuilder(blueprint.ControlPlane.InfrastructureMachineTemplate). - WithType(api.ControlPlaneInfrastructureMachineTemplateType). + WithHolder(desired.ControlPlane.Object, "spec.machineTemplate.infrastructureRef"). Build() if err != nil { return nil, errors.Wrapf(err, "failed to prepare ControlPlane's machine template %s for patching", tlog.KObj{Obj: blueprint.ControlPlane.InfrastructureMachineTemplate}) } t.Variables = controlPlaneVariables - req.Items = append(req.Items, t) + req.Items = append(req.Items, *t) } // Add BootstrapConfigTemplate and InfrastructureMachine template for all MachineDeploymentTopologies @@ -195,27 +196,25 @@ func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterStat // Add the BootstrapTemplate. t, err := newTemplateBuilder(mdClass.BootstrapTemplate). - WithType(api.MachineDeploymentBootstrapConfigTemplateType). - WithMachineDeploymentRef(mdTopology). + WithHolder(md.Object, "spec.template.spec.bootstrap.configRef"). Build() if err != nil { return nil, errors.Wrapf(err, "failed to prepare BootstrapConfig template %s for MachineDeployment topology %s for patching", tlog.KObj{Obj: mdClass.BootstrapTemplate}, mdTopologyName) } t.Variables = mdVariables - req.Items = append(req.Items, t) + req.Items = append(req.Items, *t) // Add the InfrastructureMachineTemplate. t, err = newTemplateBuilder(mdClass.InfrastructureMachineTemplate). - WithType(api.MachineDeploymentInfrastructureMachineTemplateType). - WithMachineDeploymentRef(mdTopology). + WithHolder(md.Object, "spec.template.spec.infrastructureRef"). Build() if err != nil { return nil, errors.Wrapf(err, "failed to prepare InfrastructureMachine template %s for MachineDeployment topology %s for patching", tlog.KObj{Obj: mdClass.InfrastructureMachineTemplate}, mdTopologyName) } t.Variables = mdVariables - req.Items = append(req.Items, t) + req.Items = append(req.Items, *t) } return req, nil @@ -243,20 +242,20 @@ func createPatchGenerator(patch *clusterv1.ClusterClassPatch) (api.Generator, er return nil, errors.Errorf("failed to create patch generator for patch %q", patch.Name) } -// applyPatchesToRequest updates the templates of a GenerateRequest by applying the patches -// of a GenerateResponse. -func applyPatchesToRequest(ctx context.Context, req *api.GenerateRequest, resp *api.GenerateResponse) error { +// applyPatchesToRequest updates the templates of a GeneratePatchesRequest by applying the patches +// of a GeneratePatchesResponse. +func applyPatchesToRequest(ctx context.Context, req *runtimehooksv1.GeneratePatchesRequest, resp *runtimehooksv1.GeneratePatchesResponse) error { log := tlog.LoggerFrom(ctx) for _, patch := range resp.Items { - log = log.WithValues("templateRef", patch.TemplateRef.String()) + //log = log.WithValues("templateRef", patch.Ref.String()) FIXME // Get the template the patch should be applied to. - template := getTemplate(req, patch.TemplateRef) + template := getTemplateByUID(req, patch.UID) // If a patch doesn't apply to any template, this is a misconfiguration. if template == nil { - return errors.Errorf("generated patch is targeted at the template with ID %q that does not exists", patch.TemplateRef) + return errors.Errorf("generated patch is targeted at the template with ID %q that does not exists", patch.UID) } // Use the patch to create a patched copy of the template. @@ -264,49 +263,45 @@ func applyPatchesToRequest(ctx context.Context, req *api.GenerateRequest, resp * var err error switch patch.PatchType { - case api.JSONPatchType: - log.V(5).Infof("Accumulating JSON patch: %s", string(patch.Patch.Raw)) - jsonPatch, err := jsonpatch.DecodePatch(patch.Patch.Raw) + case runtimehooksv1.JSONPatchType: + log.V(5).Infof("Accumulating JSON patch: %s", string(patch.Patch)) + jsonPatch, err := jsonpatch.DecodePatch(patch.Patch) if err != nil { return errors.Wrapf(err, "failed to apply patch to template %s: error decoding json patch (RFC6902): %s", - template.TemplateRef, string(patch.Patch.Raw)) + template.UID, string(patch.Patch)) } - patchedTemplate, err = jsonPatch.Apply(template.Template.Raw) + patchedTemplate, err = jsonPatch.Apply(template.Object.Raw) if err != nil { return errors.Wrapf(err, "failed to apply patch to template %s: error applying json patch (RFC6902): %s", - template.TemplateRef, string(patch.Patch.Raw)) + template.UID, string(patch.Patch)) } - case api.JSONMergePatchType: - log.V(5).Infof("Accumulating JSON merge patch: %s", string(patch.Patch.Raw)) - patchedTemplate, err = jsonpatch.MergePatch(template.Template.Raw, patch.Patch.Raw) + case runtimehooksv1.JSONMergePatchType: + log.V(5).Infof("Accumulating JSON merge patch: %s", string(patch.Patch)) + patchedTemplate, err = jsonpatch.MergePatch(template.Object.Raw, patch.Patch) if err != nil { return errors.Wrapf(err, "failed to apply patch to template %s: error applying json merge patch (RFC7386): %s", - template.TemplateRef, string(patch.Patch.Raw)) + template.UID, string(patch.Patch)) } } // Overwrite the spec of template.Template with the spec of the patchedTemplate, // to ensure that we only pick up changes to the spec. - if err := patchTemplateSpec(&template.Template, patchedTemplate); err != nil { + if err := patchTemplateSpec(&template.Object, patchedTemplate); err != nil { return errors.Wrapf(err, "failed to apply patch to template %s", - template.TemplateRef) + template.UID) } } return nil } -// 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. -func updateDesiredState(ctx context.Context, req *api.GenerateRequest, blueprint *scope.ClusterBlueprint, desired *scope.ClusterState) error { +// updateDesiredState uses the patched templates of a GeneratePatchesRequest to update the desired state. +// NOTE: This func should be called after all the patches have been applied to the GeneratePatchesRequest. +func updateDesiredState(ctx context.Context, req *runtimehooksv1.GeneratePatchesRequest, blueprint *scope.ClusterBlueprint, desired *scope.ClusterState) error { var err error // Update the InfrastructureCluster. - infrastructureClusterTemplate, err := getTemplateAsUnstructured(req, - newTemplateBuilder(blueprint.InfrastructureClusterTemplate). - WithType(api.InfrastructureClusterTemplateType). - BuildTemplateRef(), - ) + infrastructureClusterTemplate, err := getTemplateAsUnstructured(req, "Cluster", "spec.infrastructureRef", "") if err != nil { return err } @@ -315,11 +310,7 @@ func updateDesiredState(ctx context.Context, req *api.GenerateRequest, blueprint } // Update the ControlPlane. - controlPlaneTemplate, err := getTemplateAsUnstructured(req, - newTemplateBuilder(blueprint.ControlPlane.Template). - WithType(api.ControlPlaneTemplateType). - BuildTemplateRef(), - ) + controlPlaneTemplate, err := getTemplateAsUnstructured(req, "Cluster", "spec.controlPlaneRef", "") if err != nil { return err } @@ -336,11 +327,7 @@ func updateDesiredState(ctx context.Context, req *api.GenerateRequest, blueprint // If the ClusterClass mandates the ControlPlane has InfrastructureMachines, // update the InfrastructureMachineTemplate for ControlPlane machines. if blueprint.HasControlPlaneInfrastructureMachine() { - infrastructureMachineTemplate, err := getTemplateAsUnstructured(req, - newTemplateBuilder(blueprint.ControlPlane.InfrastructureMachineTemplate). - WithType(api.ControlPlaneInfrastructureMachineTemplateType). - BuildTemplateRef(), - ) + infrastructureMachineTemplate, err := getTemplateAsUnstructured(req, "", "spec.machineTemplate.infrastructureRef", "") if err != nil { return err } @@ -351,19 +338,8 @@ func updateDesiredState(ctx context.Context, req *api.GenerateRequest, blueprint // Update the templates for all MachineDeployments. for mdTopologyName, md := range desired.MachineDeployments { - // Lookup MachineDeploymentTopology. - mdTopology, err := lookupMDTopology(blueprint.Topology, mdTopologyName) - if err != nil { - return err - } - // Update the BootstrapConfigTemplate. - bootstrapTemplate, err := getTemplateAsUnstructured(req, - newTemplateBuilder(md.BootstrapTemplate). - WithType(api.MachineDeploymentBootstrapConfigTemplateType). - WithMachineDeploymentRef(mdTopology). - BuildTemplateRef(), - ) + bootstrapTemplate, err := getTemplateAsUnstructured(req, "MachineDeployment", "spec.template.spec.bootstrap.configRef", mdTopologyName) if err != nil { return err } @@ -372,12 +348,7 @@ func updateDesiredState(ctx context.Context, req *api.GenerateRequest, blueprint } // Update the InfrastructureMachineTemplate. - infrastructureMachineTemplate, err := getTemplateAsUnstructured(req, - newTemplateBuilder(md.InfrastructureMachineTemplate). - WithType(api.MachineDeploymentInfrastructureMachineTemplateType). - WithMachineDeploymentRef(mdTopology). - BuildTemplateRef(), - ) + infrastructureMachineTemplate, err := getTemplateAsUnstructured(req, "MachineDeployment", "spec.template.spec.infrastructureRef", mdTopologyName) if err != nil { return err } diff --git a/internal/controllers/topology/cluster/patches/engine_test.go b/internal/controllers/topology/cluster/patches/engine_test.go index 3903d2a48298..bb20645112d7 100644 --- a/internal/controllers/topology/cluster/patches/engine_test.go +++ b/internal/controllers/topology/cluster/patches/engine_test.go @@ -30,6 +30,7 @@ import ( "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/api" "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/scope" "sigs.k8s.io/cluster-api/internal/test/builder" @@ -39,7 +40,7 @@ import ( func TestApply(t *testing.T) { type patch struct { name string - patches []api.GenerateResponsePatch + patches []runtimehooksv1.GeneratePatchesResponse } type expectedFields struct { infrastructureCluster map[string]interface{} @@ -64,12 +65,12 @@ func TestApply(t *testing.T) { patches: []patch{ { name: "fake-patch1", - patches: []api.GenerateResponsePatch{ + patches: []api.GeneratePatchesResult{ { - TemplateRef: api.TemplateRef{ - APIVersion: builder.InfrastructureGroupVersion.String(), - Kind: builder.GenericInfrastructureClusterTemplateKind, - TemplateType: api.InfrastructureClusterTemplateType, + Ref: api.TemplateRef{ + APIVersion: builder.InfrastructureGroupVersion.String(), + Kind: builder.GenericInfrastructureClusterTemplateKind, + Type: api.InfrastructureClusterTemplateType, }, Patch: apiextensionsv1.JSON{Raw: []byte(` [{"op":"add","path":"/spec/template/spec/resource","value":"infraCluster"}] @@ -77,10 +78,10 @@ func TestApply(t *testing.T) { PatchType: api.JSONPatchType, }, { - TemplateRef: api.TemplateRef{ - APIVersion: builder.ControlPlaneGroupVersion.String(), - Kind: builder.GenericControlPlaneTemplateKind, - TemplateType: api.ControlPlaneTemplateType, + Ref: api.TemplateRef{ + APIVersion: builder.ControlPlaneGroupVersion.String(), + Kind: builder.GenericControlPlaneTemplateKind, + Type: api.ControlPlaneTemplateType, }, Patch: apiextensionsv1.JSON{Raw: []byte(` [{"op":"add","path":"/spec/template/spec/resource","value":"controlPlane"}] @@ -88,10 +89,10 @@ func TestApply(t *testing.T) { PatchType: api.JSONPatchType, }, { - TemplateRef: api.TemplateRef{ - APIVersion: builder.InfrastructureGroupVersion.String(), - Kind: builder.GenericInfrastructureMachineTemplateKind, - TemplateType: api.ControlPlaneInfrastructureMachineTemplateType, + Ref: api.TemplateRef{ + APIVersion: builder.InfrastructureGroupVersion.String(), + Kind: builder.GenericInfrastructureMachineTemplateKind, + Type: api.ControlPlaneInfrastructureMachineTemplateType, }, Patch: apiextensionsv1.JSON{Raw: []byte(` [{"op":"add","path":"/spec/template/spec/resource","value":"controlPlaneInfrastructureMachineTemplate"}] @@ -117,12 +118,12 @@ func TestApply(t *testing.T) { patches: []patch{ { name: "fake-patch1", - patches: []api.GenerateResponsePatch{ + patches: []api.GeneratePatchesResult{ { // Set /spec/template/spec/resource=topo1-infra in InfrastructureMachineTemplate of default-worker-topo1. - TemplateRef: api.TemplateRef{ - APIVersion: builder.InfrastructureGroupVersion.String(), - Kind: builder.GenericInfrastructureMachineTemplateKind, - TemplateType: api.MachineDeploymentInfrastructureMachineTemplateType, + Ref: api.TemplateRef{ + APIVersion: builder.InfrastructureGroupVersion.String(), + Kind: builder.GenericInfrastructureMachineTemplateKind, + Type: api.MachineDeploymentInfrastructureMachineTemplateType, MachineDeploymentRef: api.MachineDeploymentRef{ TopologyName: "default-worker-topo1", Class: "default-worker", @@ -134,10 +135,10 @@ func TestApply(t *testing.T) { PatchType: api.JSONPatchType, }, { // Set /spec/template/spec/resource=topo1-bootstrap in BootstrapTemplate of default-worker-topo1. - TemplateRef: api.TemplateRef{ - APIVersion: builder.BootstrapGroupVersion.String(), - Kind: builder.GenericBootstrapConfigTemplateKind, - TemplateType: api.MachineDeploymentBootstrapConfigTemplateType, + Ref: api.TemplateRef{ + APIVersion: builder.BootstrapGroupVersion.String(), + Kind: builder.GenericBootstrapConfigTemplateKind, + Type: api.MachineDeploymentBootstrapConfigTemplateType, MachineDeploymentRef: api.MachineDeploymentRef{ TopologyName: "default-worker-topo1", Class: "default-worker", @@ -149,10 +150,10 @@ func TestApply(t *testing.T) { PatchType: api.JSONPatchType, }, { // Set /spec/template/spec/resource=topo2-infra in InfrastructureMachineTemplate of default-worker-topo2. - TemplateRef: api.TemplateRef{ - APIVersion: builder.InfrastructureGroupVersion.String(), - Kind: builder.GenericInfrastructureMachineTemplateKind, - TemplateType: api.MachineDeploymentInfrastructureMachineTemplateType, + Ref: api.TemplateRef{ + APIVersion: builder.InfrastructureGroupVersion.String(), + Kind: builder.GenericInfrastructureMachineTemplateKind, + Type: api.MachineDeploymentInfrastructureMachineTemplateType, MachineDeploymentRef: api.MachineDeploymentRef{ TopologyName: "default-worker-topo2", Class: "default-worker", @@ -164,10 +165,10 @@ func TestApply(t *testing.T) { PatchType: api.JSONPatchType, }, { // Set /spec/template/spec/resource=topo2-bootstrap in BootstrapTemplate of default-worker-topo2. - TemplateRef: api.TemplateRef{ - APIVersion: builder.BootstrapGroupVersion.String(), - Kind: builder.GenericBootstrapConfigTemplateKind, - TemplateType: api.MachineDeploymentBootstrapConfigTemplateType, + Ref: api.TemplateRef{ + APIVersion: builder.BootstrapGroupVersion.String(), + Kind: builder.GenericBootstrapConfigTemplateKind, + Type: api.MachineDeploymentBootstrapConfigTemplateType, MachineDeploymentRef: api.MachineDeploymentRef{ TopologyName: "default-worker-topo2", Class: "default-worker", @@ -197,12 +198,12 @@ func TestApply(t *testing.T) { patches: []patch{ { name: "fake-patch1", - patches: []api.GenerateResponsePatch{ + patches: []api.GeneratePatchesResult{ { // Set /spec/template/spec/resource=infra in InfrastructureMachineTemplate of default-worker-topo1. - TemplateRef: api.TemplateRef{ - APIVersion: builder.InfrastructureGroupVersion.String(), - Kind: builder.GenericInfrastructureMachineTemplateKind, - TemplateType: api.MachineDeploymentInfrastructureMachineTemplateType, + Ref: api.TemplateRef{ + APIVersion: builder.InfrastructureGroupVersion.String(), + Kind: builder.GenericInfrastructureMachineTemplateKind, + Type: api.MachineDeploymentInfrastructureMachineTemplateType, MachineDeploymentRef: api.MachineDeploymentRef{ TopologyName: "default-worker-topo1", Class: "default-worker", @@ -214,10 +215,10 @@ func TestApply(t *testing.T) { PatchType: api.JSONPatchType, }, { // Set /spec/template/spec/resource=infra in InfrastructureMachineTemplate of default-worker-topo2. - TemplateRef: api.TemplateRef{ - APIVersion: builder.InfrastructureGroupVersion.String(), - Kind: builder.GenericInfrastructureMachineTemplateKind, - TemplateType: api.MachineDeploymentInfrastructureMachineTemplateType, + Ref: api.TemplateRef{ + APIVersion: builder.InfrastructureGroupVersion.String(), + Kind: builder.GenericInfrastructureMachineTemplateKind, + Type: api.MachineDeploymentInfrastructureMachineTemplateType, MachineDeploymentRef: api.MachineDeploymentRef{ TopologyName: "default-worker-topo2", Class: "default-worker", @@ -243,12 +244,12 @@ func TestApply(t *testing.T) { patches: []patch{ { name: "fake-patch1", - patches: []api.GenerateResponsePatch{ + patches: []api.GeneratePatchesResult{ { - TemplateRef: api.TemplateRef{ - APIVersion: builder.ControlPlaneGroupVersion.String(), - Kind: builder.GenericControlPlaneTemplateKind, - TemplateType: api.ControlPlaneTemplateType, + Ref: api.TemplateRef{ + APIVersion: builder.ControlPlaneGroupVersion.String(), + Kind: builder.GenericControlPlaneTemplateKind, + Type: api.ControlPlaneTemplateType, }, Patch: apiextensionsv1.JSON{Raw: []byte(` [{"op":"add","path":"/spec/template/spec/clusterName","value":"cluster1"}, @@ -260,12 +261,12 @@ func TestApply(t *testing.T) { }, { name: "fake-patch2", - patches: []api.GenerateResponsePatch{ + patches: []api.GeneratePatchesResult{ { - TemplateRef: api.TemplateRef{ - APIVersion: builder.ControlPlaneGroupVersion.String(), - Kind: builder.GenericControlPlaneTemplateKind, - TemplateType: api.ControlPlaneTemplateType, + Ref: api.TemplateRef{ + APIVersion: builder.ControlPlaneGroupVersion.String(), + Kind: builder.GenericControlPlaneTemplateKind, + Type: api.ControlPlaneTemplateType, }, Patch: apiextensionsv1.JSON{Raw: []byte(` [{"op":"replace","path":"/spec/template/spec/clusterName","value":"cluster1-overwritten"}] @@ -291,12 +292,12 @@ func TestApply(t *testing.T) { patches: []patch{ { name: "fake-patch1", - patches: []api.GenerateResponsePatch{ + patches: []api.GeneratePatchesResult{ { - TemplateRef: api.TemplateRef{ - APIVersion: builder.ControlPlaneGroupVersion.String(), - Kind: builder.GenericControlPlaneTemplateKind, - TemplateType: api.ControlPlaneTemplateType, + Ref: api.TemplateRef{ + APIVersion: builder.ControlPlaneGroupVersion.String(), + Kind: builder.GenericControlPlaneTemplateKind, + Type: api.ControlPlaneTemplateType, }, Patch: apiextensionsv1.JSON{Raw: []byte(` [{"op":"add","path":"/spec/template/spec/replicas","value":1}, @@ -314,12 +315,12 @@ func TestApply(t *testing.T) { patches: []patch{ { name: "fake-patch1", - patches: []api.GenerateResponsePatch{ + patches: []api.GeneratePatchesResult{ { - TemplateRef: api.TemplateRef{ - APIVersion: builder.InfrastructureGroupVersion.String(), - Kind: builder.GenericInfrastructureClusterTemplateKind, - TemplateType: api.InfrastructureClusterTemplateType, + Ref: api.TemplateRef{ + APIVersion: builder.InfrastructureGroupVersion.String(), + Kind: builder.GenericInfrastructureClusterTemplateKind, + Type: api.InfrastructureClusterTemplateType, }, Patch: apiextensionsv1.JSON{Raw: []byte(` [{"op":"add","path":"/spec/template/spec/clusterName","value":"cluster1"}, @@ -341,12 +342,12 @@ func TestApply(t *testing.T) { patches: []patch{ { name: "fake-patch1", - patches: []api.GenerateResponsePatch{ + patches: []api.GeneratePatchesResult{ { - TemplateRef: api.TemplateRef{ - APIVersion: builder.InfrastructureGroupVersion.String(), - Kind: builder.GenericInfrastructureClusterTemplateKind, - TemplateType: api.InfrastructureClusterTemplateType, + Ref: api.TemplateRef{ + APIVersion: builder.InfrastructureGroupVersion.String(), + Kind: builder.GenericInfrastructureClusterTemplateKind, + Type: api.InfrastructureClusterTemplateType, }, Patch: apiextensionsv1.JSON{Raw: []byte(` {"spec": {"template": {"spec": {"resource": "infraCluster"}}}} @@ -387,7 +388,7 @@ func TestApply(t *testing.T) { blueprint.ClusterClass.Spec.Patches = append(blueprint.ClusterClass.Spec.Patches, clusterv1.ClusterClassPatch{Name: patch.name}) } - patchEngine.createPatchGenerator = func(patch *clusterv1.ClusterClassPatch) (api.Generator, error) { + patchEngine.createPatchGenerator = func(patch *clusterv1.ClusterClassPatch) (api.TopologyMutation, error) { for _, p := range tt.patches { if p.name == patch.Name { return &fakePatchGenerator{patches: p.patches}, nil @@ -581,15 +582,15 @@ func setupTestObjects() (*scope.ClusterBlueprint, *scope.ClusterState) { return blueprint, desired } -// fakePatchGenerator is an api.Generator which just returns the provided patches. +// fakePatchGenerator is an api.TopologyMutation which just returns the provided patches. type fakePatchGenerator struct { - patches []api.GenerateResponsePatch + patches []runtimehooksv1.GeneratePatchesResponseItem } -func (g *fakePatchGenerator) Generate(_ context.Context, _ *api.GenerateRequest) (*api.GenerateResponse, error) { - return &api.GenerateResponse{ +func (g *fakePatchGenerator) Generate(_ context.Context, _ runtimehooksv1.GeneratePatchesRequest) runtimehooksv1.GeneratePatchesResponse { + return runtimehooksv1.GeneratePatchesResponse{ Items: g.patches, - }, nil + } } // setSpecFields sets fields on an unstructured object from a map. diff --git a/internal/controllers/topology/cluster/patches/inline/json_patch_generator.go b/internal/controllers/topology/cluster/patches/inline/json_patch_generator.go index aeb983397357..40f34b175ffd 100644 --- a/internal/controllers/topology/cluster/patches/inline/json_patch_generator.go +++ b/internal/controllers/topology/cluster/patches/inline/json_patch_generator.go @@ -34,11 +34,12 @@ import ( "sigs.k8s.io/yaml" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/api" patchvariables "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/variables" ) -// jsonPatchGenerator generates JSON patches for a GenerateRequest based on a ClusterClassPatch. +// jsonPatchGenerator generates JSON patches for a GeneratePatchesRequest based on a ClusterClassPatch. type jsonPatchGenerator struct { patch *clusterv1.ClusterClassPatch } @@ -50,18 +51,20 @@ func New(patch *clusterv1.ClusterClassPatch) api.Generator { } } -// Generate generates JSON patches for the given GenerateRequest based on a ClusterClassPatch. -func (j *jsonPatchGenerator) Generate(_ context.Context, req *api.GenerateRequest) (*api.GenerateResponse, error) { - resp := &api.GenerateResponse{} +// Generate generates JSON patches for the given GeneratePatchesRequest based on a ClusterClassPatch. +func (j *jsonPatchGenerator) Generate(_ context.Context, req *runtimehooksv1.GeneratePatchesRequest) *runtimehooksv1.GeneratePatchesResponse { + resp := &runtimehooksv1.GeneratePatchesResponse{} + + globalVariables := toMap(req.Variables) // Loop over all templates. errs := []error{} - for _, template := range req.Items { + for _, generatePatchesRequestItem := range req.Items { // Calculate the list of patches which match the current template. matchingPatches := []clusterv1.PatchDefinition{} for _, patch := range j.patch.Definitions { // Add the patch to the list, if it matches the template. - if templateMatchesSelector(&template.TemplateRef, patch.Selector) { + if templateMatchesSelector(&generatePatchesRequestItem, patch.Selector) { matchingPatches = append(matchingPatches, patch) } } @@ -72,15 +75,15 @@ func (j *jsonPatchGenerator) Generate(_ context.Context, req *api.GenerateReques } // Merge template-specific and global variables. - variables, err := mergeVariableMaps(req.Variables, template.Variables) + variables, err := mergeVariableMaps(globalVariables, toMap(generatePatchesRequestItem.Variables)) if err != nil { - errs = append(errs, errors.Wrapf(err, "failed to merge global and template-specific variables for template %s", template.TemplateRef)) + errs = append(errs, errors.Wrapf(err, "failed to merge global and template-specific variables for template %s", generatePatchesRequestItem.UID)) continue } enabled, err := patchIsEnabled(j.patch.EnabledIf, variables) if err != nil { - errs = append(errs, errors.Wrapf(err, "failed to calculate if patch %s is enabled for template %s", j.patch.Name, template.TemplateRef)) + errs = append(errs, errors.Wrapf(err, "failed to calculate if patch %s is enabled for template %s", j.patch.Name, generatePatchesRequestItem.UID)) continue } if !enabled { @@ -93,56 +96,78 @@ func (j *jsonPatchGenerator) Generate(_ context.Context, req *api.GenerateReques // Generate JSON patches. jsonPatches, err := generateJSONPatches(patch.JSONPatches, variables) if err != nil { - errs = append(errs, errors.Wrapf(err, "failed to generate JSON patches for template %s", template.TemplateRef)) + errs = append(errs, errors.Wrapf(err, "failed to generate JSON patches for template %s", generatePatchesRequestItem.UID)) continue } // Add jsonPatches to the response. - resp.Items = append(resp.Items, api.GenerateResponsePatch{ - TemplateRef: template.TemplateRef, - Patch: *jsonPatches, - PatchType: api.JSONPatchType, + resp.Items = append(resp.Items, runtimehooksv1.GeneratePatchesResponseItem{ + UID: generatePatchesRequestItem.UID, + Patch: jsonPatches, + PatchType: runtimehooksv1.JSONPatchType, }) } } - return resp, kerrors.NewAggregate(errs) + if err := kerrors.NewAggregate(errs); err != nil { + return &runtimehooksv1.GeneratePatchesResponse{ + Status: runtimehooksv1.ResponseStatusFailure, + Message: err.Error(), + } + } + + return resp +} + +func toMap(variables []runtimehooksv1.Variable) map[string]apiextensionsv1.JSON { + variablesMap := map[string]apiextensionsv1.JSON{} + + for i := range variables { + variablesMap[variables[i].Name] = variables[i].Value + } + return variablesMap } // templateMatchesSelector returns true if the template matches the selector. -func templateMatchesSelector(templateRef *api.TemplateRef, selector clusterv1.PatchSelector) bool { +func templateMatchesSelector(req *runtimehooksv1.GeneratePatchesRequestItem, selector clusterv1.PatchSelector) bool { + gvk := req.Object.Object.GetObjectKind().GroupVersionKind() + // Check if the apiVersion and kind are matching. - if templateRef.APIVersion != selector.APIVersion { + if gvk.GroupVersion().String() != selector.APIVersion { return false } - if templateRef.Kind != selector.Kind { + if gvk.Kind != selector.Kind { return false } - // Check if target matches. - switch templateRef.TemplateType { - case api.InfrastructureClusterTemplateType: - // Check if matchSelector.infrastructureCluster is true. - return selector.MatchResources.InfrastructureCluster - case api.ControlPlaneTemplateType, api.ControlPlaneInfrastructureMachineTemplateType: - // Check if matchSelector.controlPlane is true. - return selector.MatchResources.ControlPlane - case api.MachineDeploymentBootstrapConfigTemplateType, api.MachineDeploymentInfrastructureMachineTemplateType: - if selector.MatchResources.MachineDeploymentClass == nil { - return false + if selector.MatchResources.InfrastructureCluster { + if req.HolderReference.Kind == "Cluster" && req.HolderReference.FieldPath == "spec.infrastructureRef" { + return true + } + } + + if selector.MatchResources.ControlPlane { + if req.HolderReference.Kind == "Cluster" && req.HolderReference.FieldPath == "spec.controlPlaneRef" { + return true + } + if req.HolderReference.FieldPath == "spec.machineTemplate.infrastructureRef" { + return true } - // Check if matchSelector.machineDeploymentClass.names contains the - // MachineDeployment.Class of the template. - for _, name := range selector.MatchResources.MachineDeploymentClass.Names { - if name == templateRef.MachineDeploymentRef.Class { - return true + } + + if selector.MatchResources.MachineDeploymentClass != nil { + templateVariables := toMap(req.Variables) + + for _, mdClass := range selector.MatchResources.MachineDeploymentClass.Names { + v, err := getVariableValue(templateVariables, "builtin.machineDeployment.class") + if err != nil || string(v.Raw) != strconv.Quote(mdClass) { + continue } + return true } - return false - default: - // Return false if the TargetType is unknown. - return false } + + return false } func patchIsEnabled(enabledIf *string, variables map[string]apiextensionsv1.JSON) (bool, error) { @@ -169,7 +194,7 @@ type jsonPatchRFC6902 struct { } // generateJSONPatches generates JSON patches based on the given JSONPatches and variables. -func generateJSONPatches(jsonPatches []clusterv1.JSONPatch, variables map[string]apiextensionsv1.JSON) (*apiextensionsv1.JSON, error) { +func generateJSONPatches(jsonPatches []clusterv1.JSONPatch, variables map[string]apiextensionsv1.JSON) ([]byte, error) { res := []jsonPatchRFC6902{} for _, jsonPatch := range jsonPatches { @@ -195,7 +220,7 @@ func generateJSONPatches(jsonPatches []clusterv1.JSONPatch, variables map[string return nil, errors.Wrapf(err, "failed to marshal JSON Patch %v", jsonPatches) } - return &apiextensionsv1.JSON{Raw: resJSON}, nil + return resJSON, nil } // calculateValue calculates a value for a JSON patch. diff --git a/internal/controllers/topology/cluster/patches/inline/json_patch_generator_test.go b/internal/controllers/topology/cluster/patches/inline/json_patch_generator_test.go index 8371ee8fa0b7..760906aba950 100644 --- a/internal/controllers/topology/cluster/patches/inline/json_patch_generator_test.go +++ b/internal/controllers/topology/cluster/patches/inline/json_patch_generator_test.go @@ -27,20 +27,20 @@ import ( "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/api" patchvariables "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/variables" ) func TestGenerate(t *testing.T) { tests := []struct { - name string - patch *clusterv1.ClusterClassPatch - req *api.GenerateRequest - want *api.GenerateResponse - wantErr bool + name string + patch *clusterv1.ClusterClassPatch + req *runtimehooksv1.GeneratePatchesRequest + want *runtimehooksv1.GeneratePatchesResponse }{ { - name: "Should generate JSON Patches with correct variable values", + name: "Should generate JSON Results with correct variable values", patch: &clusterv1.ClusterClassPatch{ Name: "clusterName", Definitions: []clusterv1.PatchDefinition{ @@ -114,35 +114,31 @@ func TestGenerate(t *testing.T) { }, }, }, - req: &api.GenerateRequest{ - Variables: map[string]apiextensionsv1.JSON{ - "builtin": {Raw: []byte(`{"cluster":{"name":"cluster-name","namespace":"default","topology":{"class":"clusterClass1","version":"v1.21.1"}}}`)}, - "variableA": {Raw: []byte(`"A"`)}, - "variableB": {Raw: []byte(`"B"`)}, - "variableC": {Raw: []byte(`"C"`)}, + req: &runtimehooksv1.GeneratePatchesRequest{ + Variables: []runtimehooksv1.Variable{ + {Name: "builtin", Value: apiextensionsv1.JSON{Raw: []byte(`{"cluster":{"name":"cluster-name","namespace":"default","topology":{"class":"clusterClass1","version":"v1.21.1"}}}`)}}, + {Name: "variableA", Value: apiextensionsv1.JSON{Raw: []byte(`"A"`)}}, + {Name: "variableB", Value: apiextensionsv1.JSON{Raw: []byte(`"B"`)}}, + {Name: "variableC", Value: apiextensionsv1.JSON{Raw: []byte(`"C"`)}}, }, - Items: []*api.GenerateRequestTemplate{ + Items: []runtimehooksv1.GeneratePatchesRequestItem{ { - TemplateRef: api.TemplateRef{ - APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", - Kind: "ControlPlaneTemplate", - TemplateType: api.ControlPlaneTemplateType, + HolderReference: runtimehooksv1.HolderReference{ + APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", + Kind: "ControlPlaneTemplate", + //Type: api.ControlPlaneTemplateType,TODO }, - Variables: map[string]apiextensionsv1.JSON{ - "builtin": {Raw: []byte(`{"controlPlane":{"replicas":3,"machineTemplate":{"infrastructureRef":{"name":"controlPlaneInfrastructureMachineTemplate1"}}}}`)}, - "variableC": {Raw: []byte(`"C-template"`)}, + Variables: []runtimehooksv1.Variable{ + {Name: "builtin", Value: apiextensionsv1.JSON{Raw: []byte(`{"controlPlane":{"replicas":3,"machineTemplate":{"infrastructureRef":{"name":"controlPlaneInfrastructureMachineTemplate1"}}}}`)}}, + {Name: "variableC", Value: apiextensionsv1.JSON{Raw: []byte(`"C-template"`)}}, }, }, }, }, - want: &api.GenerateResponse{ - Items: []api.GenerateResponsePatch{ + want: &runtimehooksv1.GeneratePatchesResponse{ + Items: []runtimehooksv1.GeneratePatchesResponseItem{ { - TemplateRef: api.TemplateRef{ - APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", - Kind: "ControlPlaneTemplate", - TemplateType: api.ControlPlaneTemplateType, - }, + UID: "123", // TODO Patch: toJSONCompact(`[ {"op":"replace","path":"/spec/value","value":1}, {"op":"replace","path":"/spec/valueFrom/variable","value":"A"}, @@ -158,13 +154,13 @@ func TestGenerate(t *testing.T) { } } }]}]`), - PatchType: api.JSONPatchType, + PatchType: runtimehooksv1.JSONPatchType, }, }, }, }, { - name: "Should generate JSON Patches (multiple PatchDefinitions)", + name: "Should generate JSON Results (multiple PatchDefinitions)", patch: &clusterv1.ClusterClassPatch{ Name: "clusterName", Definitions: []clusterv1.PatchDefinition{ @@ -234,58 +230,47 @@ func TestGenerate(t *testing.T) { }, }, }, - req: &api.GenerateRequest{ - Variables: map[string]apiextensionsv1.JSON{ - "builtin": {Raw: []byte(`{"cluster":{"name":"cluster-name","namespace":"default","topology":{"class":"clusterClass1","version":"v1.21.1"}}}`)}, + req: &runtimehooksv1.GeneratePatchesRequest{ + Variables: []runtimehooksv1.Variable{ + {Name: "builtin", Value: apiextensionsv1.JSON{Raw: []byte(`{"cluster":{"name":"cluster-name","namespace":"default","topology":{"class":"clusterClass1","version":"v1.21.1"}}}`)}}, }, - Items: []*api.GenerateRequestTemplate{ + Items: []runtimehooksv1.GeneratePatchesRequestItem{ { - TemplateRef: api.TemplateRef{ - APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", - Kind: "ControlPlaneTemplate", - TemplateType: api.ControlPlaneTemplateType, + HolderReference: runtimehooksv1.HolderReference{ + APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", + Kind: "ControlPlaneTemplate", + //Type: api.ControlPlaneTemplateType, TODO }, }, { - TemplateRef: api.TemplateRef{ - APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1", - Kind: "BootstrapTemplate", - TemplateType: api.MachineDeploymentBootstrapConfigTemplateType, - MachineDeploymentRef: api.MachineDeploymentRef{ - Class: "default-worker", - }, + HolderReference: runtimehooksv1.HolderReference{ + APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1", + Kind: "BootstrapTemplate", + //Type: api.MachineDeploymentBootstrapConfigTemplateType, + //MachineDeploymentRef: api.MachineDeploymentRef{ + // Class: "default-worker", + //}, TODO }, }, }, }, - want: &api.GenerateResponse{ - Items: []api.GenerateResponsePatch{ + want: &runtimehooksv1.GeneratePatchesResponse{ + Items: []runtimehooksv1.GeneratePatchesResponseItem{ { - TemplateRef: api.TemplateRef{ - APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", - Kind: "ControlPlaneTemplate", - TemplateType: api.ControlPlaneTemplateType, - }, + UID: "123", // TODO Patch: toJSONCompact(`[ {"op":"replace","path":"/spec/template/spec/kubeadmConfigSpec/clusterConfiguration/controllerManager/extraArgs/cluster-name","value":"cluster-name"}, {"op":"replace","path":"/spec/template/spec/kubeadmConfigSpec/files","value":[{"contentFrom":{"secret":{"key":"control-plane-azure.json","name":"cluster-name-control-plane-azure-json"}},"owner":"root:root"}]}, {"op":"remove","path":"/spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/extraArgs"} ]`), - PatchType: api.JSONPatchType, + PatchType: runtimehooksv1.JSONPatchType, }, { - TemplateRef: api.TemplateRef{ - APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1", - Kind: "BootstrapTemplate", - TemplateType: api.MachineDeploymentBootstrapConfigTemplateType, - MachineDeploymentRef: api.MachineDeploymentRef{ - Class: "default-worker", - }, - }, + UID: "123", // TODO Patch: toJSONCompact(`[ {"op":"replace","path":"/spec/template/spec/kubeadmConfigSpec/clusterConfiguration/controllerManager/extraArgs/cluster-name","value":[{"contentFrom":{"secret":{"key":"worker-node-azure.json","name":"cluster-name-md-0-azure-json"}},"owner":"root:root"}]} ]`), - PatchType: api.JSONPatchType, + PatchType: runtimehooksv1.JSONPatchType, }, }, }, @@ -296,12 +281,7 @@ func TestGenerate(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - got, err := New(tt.patch).Generate(context.Background(), tt.req) - if tt.wantErr { - g.Expect(err).To(HaveOccurred()) - return - } - g.Expect(err).ToNot(HaveOccurred()) + got := New(tt.patch).GeneratePatches(context.Background(), tt.req) g.Expect(got).To(Equal(tt.want)) }) @@ -311,7 +291,7 @@ func TestGenerate(t *testing.T) { func TestTemplateMatchesSelector(t *testing.T) { tests := []struct { name string - templateRef *api.TemplateRef + templateRef *runtimehooksv1.HolderReference selector clusterv1.PatchSelector match bool }{ @@ -342,9 +322,9 @@ func TestTemplateMatchesSelector(t *testing.T) { { name: "Match InfrastructureClusterTemplate", templateRef: &api.TemplateRef{ - APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", - Kind: "AzureClusterTemplate", - TemplateType: api.InfrastructureClusterTemplateType, + APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", + Kind: "AzureClusterTemplate", + Type: api.InfrastructureClusterTemplateType, }, selector: clusterv1.PatchSelector{ APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", @@ -358,9 +338,9 @@ func TestTemplateMatchesSelector(t *testing.T) { { name: "Don't match InfrastructureClusterTemplate, .matchResources.infrastructureCluster not set", templateRef: &api.TemplateRef{ - APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", - Kind: "AzureClusterTemplate", - TemplateType: api.InfrastructureClusterTemplateType, + APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", + Kind: "AzureClusterTemplate", + Type: api.InfrastructureClusterTemplateType, }, selector: clusterv1.PatchSelector{ APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", @@ -372,9 +352,9 @@ func TestTemplateMatchesSelector(t *testing.T) { { name: "Don't match InfrastructureClusterTemplate, .matchResources.infrastructureCluster false", templateRef: &api.TemplateRef{ - APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", - Kind: "AzureClusterTemplate", - TemplateType: api.InfrastructureClusterTemplateType, + APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", + Kind: "AzureClusterTemplate", + Type: api.InfrastructureClusterTemplateType, }, selector: clusterv1.PatchSelector{ APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", @@ -388,9 +368,9 @@ func TestTemplateMatchesSelector(t *testing.T) { { name: "Match ControlPlaneTemplate", templateRef: &api.TemplateRef{ - APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", - Kind: "ControlPlaneTemplate", - TemplateType: api.ControlPlaneTemplateType, + APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", + Kind: "ControlPlaneTemplate", + Type: api.ControlPlaneTemplateType, }, selector: clusterv1.PatchSelector{ APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", @@ -404,9 +384,9 @@ func TestTemplateMatchesSelector(t *testing.T) { { name: "Don't match ControlPlaneTemplate, .matchResources.controlPlane not set", templateRef: &api.TemplateRef{ - APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", - Kind: "ControlPlaneTemplate", - TemplateType: api.ControlPlaneTemplateType, + APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", + Kind: "ControlPlaneTemplate", + Type: api.ControlPlaneTemplateType, }, selector: clusterv1.PatchSelector{ APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", @@ -418,9 +398,9 @@ func TestTemplateMatchesSelector(t *testing.T) { { name: "Don't match ControlPlaneTemplate, .matchResources.controlPlane false", templateRef: &api.TemplateRef{ - APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", - Kind: "ControlPlaneTemplate", - TemplateType: api.ControlPlaneTemplateType, + APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", + Kind: "ControlPlaneTemplate", + Type: api.ControlPlaneTemplateType, }, selector: clusterv1.PatchSelector{ APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", @@ -434,9 +414,9 @@ func TestTemplateMatchesSelector(t *testing.T) { { name: "Match ControlPlane InfrastructureMachineTemplate", templateRef: &api.TemplateRef{ - APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", - Kind: "AzureMachineTemplate", - TemplateType: api.ControlPlaneInfrastructureMachineTemplateType, + APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", + Kind: "AzureMachineTemplate", + Type: api.ControlPlaneInfrastructureMachineTemplateType, }, selector: clusterv1.PatchSelector{ APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", @@ -450,9 +430,9 @@ func TestTemplateMatchesSelector(t *testing.T) { { name: "Match MD BootstrapTemplate", templateRef: &api.TemplateRef{ - APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1", - Kind: "BootstrapTemplate", - TemplateType: api.MachineDeploymentBootstrapConfigTemplateType, + APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1", + Kind: "BootstrapTemplate", + Type: api.MachineDeploymentBootstrapConfigTemplateType, MachineDeploymentRef: api.MachineDeploymentRef{ Class: "classA", }, @@ -471,9 +451,9 @@ func TestTemplateMatchesSelector(t *testing.T) { { name: "Don't match BootstrapTemplate, .matchResources.machineDeploymentClass not set", templateRef: &api.TemplateRef{ - APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1", - Kind: "BootstrapTemplate", - TemplateType: api.MachineDeploymentBootstrapConfigTemplateType, + APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1", + Kind: "BootstrapTemplate", + Type: api.MachineDeploymentBootstrapConfigTemplateType, MachineDeploymentRef: api.MachineDeploymentRef{ Class: "classA", }, @@ -488,9 +468,9 @@ func TestTemplateMatchesSelector(t *testing.T) { { name: "Don't match BootstrapTemplate, .matchResources.machineDeploymentClass does not match", templateRef: &api.TemplateRef{ - APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1", - Kind: "BootstrapTemplate", - TemplateType: api.MachineDeploymentBootstrapConfigTemplateType, + APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1", + Kind: "BootstrapTemplate", + Type: api.MachineDeploymentBootstrapConfigTemplateType, MachineDeploymentRef: api.MachineDeploymentRef{ Class: "classA", }, @@ -509,9 +489,9 @@ func TestTemplateMatchesSelector(t *testing.T) { { name: "Match MD InfrastructureMachineTemplate", templateRef: &api.TemplateRef{ - APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", - Kind: "AzureMachineTemplate", - TemplateType: api.MachineDeploymentInfrastructureMachineTemplateType, + APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", + Kind: "AzureMachineTemplate", + Type: api.MachineDeploymentInfrastructureMachineTemplateType, MachineDeploymentRef: api.MachineDeploymentRef{ Class: "classA", }, @@ -532,8 +512,8 @@ func TestTemplateMatchesSelector(t *testing.T) { templateRef: &api.TemplateRef{ APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", Kind: "ControlPlaneTemplate", - // invalid is an invalid TemplateType. - TemplateType: "invalid", + // invalid is an invalid Type. + Type: "invalid", }, selector: clusterv1.PatchSelector{ APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", @@ -1653,10 +1633,10 @@ func TestMergeVariables(t *testing.T) { } // toJSONCompact is used to be able to write JSON values in a readable manner. -func toJSONCompact(value string) apiextensionsv1.JSON { +func toJSONCompact(value string) []byte { var compactValue bytes.Buffer if err := json.Compact(&compactValue, []byte(value)); err != nil { panic(err) } - return apiextensionsv1.JSON{Raw: compactValue.Bytes()} + return compactValue.Bytes() } diff --git a/internal/controllers/topology/cluster/patches/patch.go b/internal/controllers/topology/cluster/patches/patch.go index f880bc4651a4..29ce0fd56a65 100644 --- a/internal/controllers/topology/cluster/patches/patch.go +++ b/internal/controllers/topology/cluster/patches/patch.go @@ -24,8 +24,8 @@ import ( jsonpatch "github.com/evanphx/json-patch/v5" "github.com/pkg/errors" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/cluster-api/internal/contract" tlog "sigs.k8s.io/cluster-api/internal/log" @@ -135,7 +135,7 @@ func calculateDiff(original, patched *unstructured.Unstructured) ([]byte, error) } // patchTemplateSpec overwrites spec in templateJSON with spec of patchedTemplateBytes. -func patchTemplateSpec(templateJSON *apiextensionsv1.JSON, patchedTemplateBytes []byte) error { +func patchTemplateSpec(templateJSON *runtime.RawExtension, patchedTemplateBytes []byte) error { // Convert templates to Unstructured. template, err := bytesToUnstructured(templateJSON.Raw) if err != nil { @@ -161,6 +161,7 @@ func patchTemplateSpec(templateJSON *apiextensionsv1.JSON, patchedTemplateBytes if err != nil { return errors.Wrapf(err, "failed to marshal patched template") } + templateJSON.Object = template templateJSON.Raw = templateBytes return nil } diff --git a/internal/controllers/topology/cluster/patches/template.go b/internal/controllers/topology/cluster/patches/template.go index 01b9f616eb53..e86292657298 100644 --- a/internal/controllers/topology/cluster/patches/template.go +++ b/internal/controllers/topology/cluster/patches/template.go @@ -18,21 +18,26 @@ package patches import ( "encoding/json" + "strconv" + "strings" "github.com/pkg/errors" + "github.com/valyala/fastjson" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/uuid" + "k8s.io/utils/pointer" + "sigs.k8s.io/controller-runtime/pkg/client" - clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/api" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" ) // templateBuilder builds templates. type templateBuilder struct { - template *unstructured.Unstructured - templateType api.TemplateType - mdTopology *clusterv1.MachineDeploymentTopology + template *unstructured.Unstructured + holder runtimehooksv1.HolderReference } // newTemplateBuilder returns a new templateBuilder. @@ -42,98 +47,92 @@ func newTemplateBuilder(template *unstructured.Unstructured) *templateBuilder { } } -// WithType adds templateType to the templateBuilder. -func (t *templateBuilder) WithType(templateType api.TemplateType) *templateBuilder { - t.templateType = templateType - return t -} - -// WithMachineDeploymentRef adds a MachineDeploymentTopology to the templateBuilder, -// which is used to add a MachineDeploymentRef to the TemplateRef during BuildTemplateRef. -func (t *templateBuilder) WithMachineDeploymentRef(mdTopology *clusterv1.MachineDeploymentTopology) *templateBuilder { - t.mdTopology = mdTopology - return t -} - -// BuildTemplateRef builds an api.TemplateRef. -func (t *templateBuilder) BuildTemplateRef() api.TemplateRef { - templateRef := api.TemplateRef{ - APIVersion: t.template.GetAPIVersion(), - Kind: t.template.GetKind(), - TemplateType: t.templateType, +// WithHolder adds holder to the templateBuilder. +func (t *templateBuilder) WithHolder(object client.Object, fieldPath string) *templateBuilder { + t.holder = runtimehooksv1.HolderReference{ + APIVersion: object.GetObjectKind().GroupVersionKind().GroupKind().String(), + Kind: object.GetObjectKind().GroupVersionKind().Kind, + Namespace: object.GetNamespace(), + Name: object.GetName(), + FieldPath: fieldPath, } - - if t.mdTopology != nil { - templateRef.MachineDeploymentRef = api.MachineDeploymentRef{ - TopologyName: t.mdTopology.Name, - Class: t.mdTopology.Class, - } - } - - return templateRef + return t } -// Build builds an api.GenerateRequestTemplate. -func (t *templateBuilder) Build() (*api.GenerateRequestTemplate, error) { - tpl := &api.GenerateRequestTemplate{ - TemplateRef: t.BuildTemplateRef(), +// Build builds an runtimehooksv1.GeneratePatchesRequestItem. +func (t *templateBuilder) Build() (*runtimehooksv1.GeneratePatchesRequestItem, error) { + tpl := &runtimehooksv1.GeneratePatchesRequestItem{ + HolderReference: t.holder, + UID: uuid.NewUUID(), } jsonObj, err := json.Marshal(t.template) if err != nil { return nil, errors.Wrap(err, "failed to marshal object to json") } - tpl.Template = apiextensionsv1.JSON{Raw: jsonObj} + // FIXME: verify if runtime.RawExtension works everywhere. + tpl.Object = runtime.RawExtension{ + Raw: jsonObj, + Object: t.template, + } return tpl, nil } -// getTemplateAsUnstructured is a utility func that returns a template matching the templateRef from a GenerateRequest. -func getTemplateAsUnstructured(req *api.GenerateRequest, templateRef api.TemplateRef) (*unstructured.Unstructured, error) { +// getTemplateAsUnstructured is a utility func that returns a template matching the templateRef from a GeneratePatchesRequest. +func getTemplateAsUnstructured(req *runtimehooksv1.GeneratePatchesRequest, holderKind, holderFieldPath, mdTopologyName string) (*unstructured.Unstructured, error) { // Find the template the patch should be applied to. - template := getTemplate(req, templateRef) + template := getTemplate(req, holderKind, holderFieldPath, mdTopologyName) // If a patch doesn't apply to any object, this is a misconfiguration. if template == nil { - return nil, errors.Errorf("failed to get template %s", templateRef) + return nil, errors.Errorf("failed to get template") } return templateToUnstructured(template) } -// getTemplate is a utility func that returns a template matching the templateRef from a GenerateRequest. -func getTemplate(req *api.GenerateRequest, templateRef api.TemplateRef) *api.GenerateRequestTemplate { - for _, template := range req.Items { - if templateRefsAreEqual(templateRef, template.TemplateRef) { - return template +// getTemplateByUID is a utility func that returns a template matching the templateRef from a GeneratePatchesRequest. +func getTemplateByUID(req *runtimehooksv1.GeneratePatchesRequest, templateRef types.UID) *runtimehooksv1.GeneratePatchesRequestItem { + for i, _ := range req.Items { + if req.Items[i].UID == templateRef { + return &req.Items[i] } } return nil } -// templateRefsAreEqual returns true if the TemplateRefs are equal. -func templateRefsAreEqual(a, b api.TemplateRef) bool { - return a.APIVersion == b.APIVersion && - a.Kind == b.Kind && - a.TemplateType == b.TemplateType && - a.MachineDeploymentRef.TopologyName == b.MachineDeploymentRef.TopologyName && - a.MachineDeploymentRef.Class == b.MachineDeploymentRef.Class +// getTemplateByHolder is a utility func that returns a template matching the templateRef from a GeneratePatchesRequest. +func getTemplate(req *runtimehooksv1.GeneratePatchesRequest, holderKind, holderFieldPath, mdTopologyName string) *runtimehooksv1.GeneratePatchesRequestItem { + for _, template := range req.Items { + if holderKind != "" && template.HolderReference.Kind != holderKind { + continue + } + if holderFieldPath != "" && template.HolderReference.FieldPath != holderFieldPath { + continue + } + if mdTopologyName != "" { + templateVariables := toMap(template.Variables) + + v, err := getVariableValue(templateVariables, "builtin.machineDeployment.topologyName") + if err != nil || string(v.Raw) != strconv.Quote(mdTopologyName) { + continue + } + } + + return &template + } + return nil } -// templateToUnstructured converts a GenerateRequestTemplate into an Unstructured object. -func templateToUnstructured(t *api.GenerateRequestTemplate) (*unstructured.Unstructured, error) { +// templateToUnstructured converts a GeneratePatchesRequestItem into an Unstructured object. +func templateToUnstructured(t *runtimehooksv1.GeneratePatchesRequestItem) (*unstructured.Unstructured, error) { // Unmarshal the template. - u, err := bytesToUnstructured(t.Template.Raw) + u, err := bytesToUnstructured(t.Object.Raw) if err != nil { return nil, errors.Wrapf(err, "failed to convert template to Unstructured") } - // Set the GVK. - gv, err := schema.ParseGroupVersion(t.TemplateRef.APIVersion) - if err != nil { - return nil, errors.Wrap(err, "failed to convert template to Unstructured: failed to parse group version") - } - u.SetGroupVersionKind(gv.WithKind(t.TemplateRef.Kind)) return u, nil } @@ -151,3 +150,149 @@ func bytesToUnstructured(b []byte) (*unstructured.Unstructured, error) { return u, nil } + +// FIXME: dedup +func toMap(variables []runtimehooksv1.Variable) map[string]apiextensionsv1.JSON { + variablesMap := map[string]apiextensionsv1.JSON{} + + for i := range variables { + variablesMap[variables[i].Name] = variables[i].Value + } + return variablesMap +} + +const ( + leftArrayDelim = "[" + rightArrayDelim = "]" +) + +// getVariableValue returns a variable from the variables map. +func getVariableValue(variables map[string]apiextensionsv1.JSON, variablePath string) (*apiextensionsv1.JSON, error) { + // Split the variablePath (format: "."). + variableSplit := strings.Split(variablePath, ".") + variableName, relativePath := variableSplit[0], variableSplit[1:] + + // Parse the path segment. + variableNameSegment, err := parsePathSegment(variableName) + if err != nil { + return nil, errors.Wrapf(err, "variable %q is invalid", variablePath) + } + + // Get the variable. + value, ok := variables[variableNameSegment.path] + if !ok { + return nil, errors.Errorf("variable %q does not exist", variableName) + } + + // Return the value, if variablePath points to a top-level variable, i.e. hos no relativePath and no + // array index (i.e. ""). + if len(relativePath) == 0 && !variableNameSegment.HasIndex() { + return &value, nil + } + + // Parse the variable object. + variable, err := fastjson.ParseBytes(value.Raw) + if err != nil { + return nil, errors.Wrapf(err, "cannot parse variable %q: %s", variableName, string(value.Raw)) + } + + // If variableName contains an array index, get the array element (i.e. starts with "[i]"). + // Then return it, if there is no relative path (i.e. "[i]") + if variableNameSegment.HasIndex() { + variable, err = getVariableArrayElement(variable, variableNameSegment, variablePath) + if err != nil { + return nil, err + } + + if len(relativePath) == 0 { + return &apiextensionsv1.JSON{ + Raw: variable.MarshalTo([]byte{}), + }, nil + } + } + + // If the variablePath points to a nested variable, i.e. has a relativePath, inspect the variable object. + + // Retrieve each segment of the relativePath incrementally, taking care of resolving array indexes. + for _, p := range relativePath { + // Parse the path segment. + pathSegment, err := parsePathSegment(p) + if err != nil { + return nil, errors.Wrapf(err, "variable %q has invalid syntax", variablePath) + } + + // Return if the variable does not exist. + if !variable.Exists(pathSegment.path) { + return nil, errors.Errorf("variable %q does not exist: failed to lookup segment %q", variablePath, pathSegment.path) + } + + // Get the variable from the variable object. + variable = variable.Get(pathSegment.path) + + // Continue if the path doesn't contain an index. + if !pathSegment.HasIndex() { + continue + } + + variable, err = getVariableArrayElement(variable, pathSegment, variablePath) + if err != nil { + return nil, err + } + } + + // Return the marshalled value of the variable. + return &apiextensionsv1.JSON{ + Raw: variable.MarshalTo([]byte{}), + }, nil +} + +type pathSegment struct { + path string + index *int +} + +func (p pathSegment) HasIndex() bool { + return p.index != nil +} + +func parsePathSegment(segment string) (*pathSegment, error) { + if (strings.Contains(segment, leftArrayDelim) && !strings.Contains(segment, rightArrayDelim)) || + (!strings.Contains(segment, leftArrayDelim) && strings.Contains(segment, rightArrayDelim)) { + return nil, errors.Errorf("failed to parse path segment %q", segment) + } + + if !strings.Contains(segment, leftArrayDelim) && !strings.Contains(segment, rightArrayDelim) { + return &pathSegment{ + path: segment, + }, nil + } + + arrayIndexStr := segment[strings.Index(segment, leftArrayDelim)+1 : strings.Index(segment, rightArrayDelim)] + index, err := strconv.Atoi(arrayIndexStr) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse array index in path segment %q", segment) + } + if index < 0 { + return nil, errors.Errorf("invalid array index %d in path segment %q", index, segment) + } + + return &pathSegment{ + path: segment[:strings.Index(segment, leftArrayDelim)], //nolint:gocritic // We already check above that segment contains leftArrayDelim, + index: pointer.Int(index), + }, nil +} + +// getVariableArrayElement gets the array element of a given array. +func getVariableArrayElement(array *fastjson.Value, arrayPathSegment *pathSegment, fullVariablePath string) (*fastjson.Value, error) { + // Retrieve the array element, handling index out of range. + arr, err := array.Array() + if err != nil { + return nil, errors.Wrapf(err, "variable %q is invalid: failed to get array %q", fullVariablePath, arrayPathSegment.path) + } + + if len(arr) < *arrayPathSegment.index+1 { + return nil, errors.Errorf("variable %q is invalid: array does not have index %d", fullVariablePath, arrayPathSegment.index) + } + + return arr[*arrayPathSegment.index], nil +} diff --git a/internal/controllers/topology/cluster/patches/variables/variables.go b/internal/controllers/topology/cluster/patches/variables/variables.go index 2140163e8a8e..9c823b98eb2d 100644 --- a/internal/controllers/topology/cluster/patches/variables/variables.go +++ b/internal/controllers/topology/cluster/patches/variables/variables.go @@ -26,6 +26,7 @@ import ( "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" "sigs.k8s.io/cluster-api/internal/contract" ) @@ -166,18 +167,17 @@ type MachineDeploymentInfrastructureRefBuiltins struct { Name string `json:"name,omitempty"` } -// VariableMap is a name/value map of variables. -// Values are marshalled as JSON. -type VariableMap map[string]apiextensionsv1.JSON - // Global returns variables that apply to all the templates, including user provided variables // and builtin variables for the Cluster object. -func Global(clusterTopology *clusterv1.Topology, cluster *clusterv1.Cluster) (VariableMap, error) { - variables := VariableMap{} +func Global(clusterTopology *clusterv1.Topology, cluster *clusterv1.Cluster) ([]runtimehooksv1.Variable, error) { + variables := []runtimehooksv1.Variable{} // Add user defined variables from Cluster.spec.topology.variables. for _, variable := range clusterTopology.Variables { - variables[variable.Name] = variable.Value + variables = append(variables, runtimehooksv1.Variable{ + Name: variable.Name, + Value: variable.Value, + }) } // Construct builtin variable. @@ -211,16 +211,18 @@ func Global(clusterTopology *clusterv1.Topology, cluster *clusterv1.Cluster) (Va } // Add builtin variables derived from the cluster object. - if err := setVariable(variables, BuiltinsName, builtin); err != nil { + variable, err := toVariable(BuiltinsName, builtin) + if err != nil { return nil, err } + variables = append(variables, *variable) return variables, nil } // ControlPlane returns variables that apply to templates belonging to the ControlPlane. -func ControlPlane(cpTopology *clusterv1.ControlPlaneTopology, cp, cpInfrastructureMachineTemplate *unstructured.Unstructured) (VariableMap, error) { - variables := VariableMap{} +func ControlPlane(cpTopology *clusterv1.ControlPlaneTopology, cp, cpInfrastructureMachineTemplate *unstructured.Unstructured) ([]runtimehooksv1.Variable, error) { + variables := []runtimehooksv1.Variable{} // Construct builtin variable. builtin := Builtins{ @@ -255,21 +257,26 @@ func ControlPlane(cpTopology *clusterv1.ControlPlaneTopology, cp, cpInfrastructu } } - if err := setVariable(variables, BuiltinsName, builtin); err != nil { + variable, err := toVariable(BuiltinsName, builtin) + if err != nil { return nil, err } + variables = append(variables, *variable) return variables, nil } // MachineDeployment returns variables that apply to templates belonging to a MachineDeployment. -func MachineDeployment(mdTopology *clusterv1.MachineDeploymentTopology, md *clusterv1.MachineDeployment, mdBootstrapTemplate, mdInfrastructureMachineTemplate *unstructured.Unstructured) (VariableMap, error) { - variables := VariableMap{} +func MachineDeployment(mdTopology *clusterv1.MachineDeploymentTopology, md *clusterv1.MachineDeployment, mdBootstrapTemplate, mdInfrastructureMachineTemplate *unstructured.Unstructured) ([]runtimehooksv1.Variable, error) { + variables := []runtimehooksv1.Variable{} // Add variables overrides for the MachineDeployment. if mdTopology.Variables != nil { for _, variable := range mdTopology.Variables.Overrides { - variables[variable.Name] = variable.Value + variables = append(variables, runtimehooksv1.Variable{ + Name: variable.Name, + Value: variable.Value, + }) } } @@ -300,22 +307,26 @@ func MachineDeployment(mdTopology *clusterv1.MachineDeploymentTopology, md *clus } } - if err := setVariable(variables, BuiltinsName, builtin); err != nil { + variable, err := toVariable(BuiltinsName, builtin) + if err != nil { return nil, err } + variables = append(variables, *variable) return variables, nil } -// setVariable converts value to JSON and adds the variable to the variables map. -func setVariable(variables VariableMap, name string, value interface{}) error { +// toVariable converts name and value to a variable. +func toVariable(name string, value interface{}) (*runtimehooksv1.Variable, error) { marshalledValue, err := json.Marshal(value) if err != nil { - return errors.Wrapf(err, "failed to set variable %q: error marshalling", name) + return nil, errors.Wrapf(err, "failed to set variable %q: error marshalling", name) } - variables[name] = apiextensionsv1.JSON{Raw: marshalledValue} - return nil + return &runtimehooksv1.Variable{ + Name: name, + Value: apiextensionsv1.JSON{Raw: marshalledValue}, + }, nil } func ipFamilyToString(ipFamily clusterv1.ClusterIPFamily) string {