diff --git a/api/v1beta1/clusterclass_types.go b/api/v1beta1/clusterclass_types.go index 6b0d37aae40f..8a5a53f3cfdb 100644 --- a/api/v1beta1/clusterclass_types.go +++ b/api/v1beta1/clusterclass_types.go @@ -338,7 +338,12 @@ type ClusterClassPatch struct { // Definitions define the patches inline. // Note: Patches will be applied in the order of the array. - Definitions []PatchDefinition `json:"definitions"` + // +optional + Definitions []PatchDefinition `json:"definitions,omitempty"` + + // External defines an external patch. + // +optional + External *ExternalPatchDefinition `json:"external,omitempty"` } // PatchDefinition defines a patch which is applied to customize the referenced templates. @@ -441,6 +446,17 @@ type JSONPatchValue struct { Template *string `json:"template,omitempty"` } +// ExternalPatchDefinition defines an external patch. +type ExternalPatchDefinition struct { + // GenerateExtension references an extension which is called to generate patches. + // +optional + GenerateExtension *string `json:"generateExtension,omitempty"` + + // ValidateExtension references an extension which is called to validate the topology. + // +optional + ValidateExtension *string `json:"validateExtension,omitempty"` +} + // LocalObjectTemplate defines a template for a topology Class. type LocalObjectTemplate struct { // Ref is a required reference to a custom resource diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index a35644d945a2..db145ed1c65e 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -170,6 +170,11 @@ func (in *ClusterClassPatch) DeepCopyInto(out *ClusterClassPatch) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.External != nil { + in, out := &in.External, &out.External + *out = new(ExternalPatchDefinition) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterClassPatch. @@ -473,6 +478,31 @@ func (in *ControlPlaneTopology) DeepCopy() *ControlPlaneTopology { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ExternalPatchDefinition) DeepCopyInto(out *ExternalPatchDefinition) { + *out = *in + if in.GenerateExtension != nil { + in, out := &in.GenerateExtension, &out.GenerateExtension + *out = new(string) + **out = **in + } + if in.ValidateExtension != nil { + in, out := &in.ValidateExtension, &out.ValidateExtension + *out = new(string) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalPatchDefinition. +func (in *ExternalPatchDefinition) DeepCopy() *ExternalPatchDefinition { + if in == nil { + return nil + } + out := new(ExternalPatchDefinition) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *FailureDomainSpec) DeepCopyInto(out *FailureDomainSpec) { *out = *in diff --git a/api/v1beta1/zz_generated.openapi.go b/api/v1beta1/zz_generated.openapi.go index 7a9e1fea8d40..885c378af498 100644 --- a/api/v1beta1/zz_generated.openapi.go +++ b/api/v1beta1/zz_generated.openapi.go @@ -46,6 +46,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "sigs.k8s.io/cluster-api/api/v1beta1.Condition": schema_sigsk8sio_cluster_api_api_v1beta1_Condition(ref), "sigs.k8s.io/cluster-api/api/v1beta1.ControlPlaneClass": schema_sigsk8sio_cluster_api_api_v1beta1_ControlPlaneClass(ref), "sigs.k8s.io/cluster-api/api/v1beta1.ControlPlaneTopology": schema_sigsk8sio_cluster_api_api_v1beta1_ControlPlaneTopology(ref), + "sigs.k8s.io/cluster-api/api/v1beta1.ExternalPatchDefinition": schema_sigsk8sio_cluster_api_api_v1beta1_ExternalPatchDefinition(ref), "sigs.k8s.io/cluster-api/api/v1beta1.FailureDomainSpec": schema_sigsk8sio_cluster_api_api_v1beta1_FailureDomainSpec(ref), "sigs.k8s.io/cluster-api/api/v1beta1.JSONPatch": schema_sigsk8sio_cluster_api_api_v1beta1_JSONPatch(ref), "sigs.k8s.io/cluster-api/api/v1beta1.JSONPatchValue": schema_sigsk8sio_cluster_api_api_v1beta1_JSONPatchValue(ref), @@ -328,12 +329,18 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_ClusterClassPatch(ref common.Refer }, }, }, + "external": { + SchemaProps: spec.SchemaProps{ + Description: "External defines an external patch.", + Ref: ref("sigs.k8s.io/cluster-api/api/v1beta1.ExternalPatchDefinition"), + }, + }, }, - Required: []string{"name", "definitions"}, + Required: []string{"name"}, }, }, Dependencies: []string{ - "sigs.k8s.io/cluster-api/api/v1beta1.PatchDefinition"}, + "sigs.k8s.io/cluster-api/api/v1beta1.ExternalPatchDefinition", "sigs.k8s.io/cluster-api/api/v1beta1.PatchDefinition"}, } } @@ -838,6 +845,33 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_ControlPlaneTopology(ref common.Re } } +func schema_sigsk8sio_cluster_api_api_v1beta1_ExternalPatchDefinition(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "ExternalPatchDefinition defines an external patch.", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "generateExtension": { + SchemaProps: spec.SchemaProps{ + Description: "GenerateExtension references an extension which is called to generate patches.", + Type: []string{"string"}, + Format: "", + }, + }, + "validateExtension": { + SchemaProps: spec.SchemaProps{ + Description: "ValidateExtension references an extension which is called to validate the topology.", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + } +} + func schema_sigsk8sio_cluster_api_api_v1beta1_FailureDomainSpec(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ diff --git a/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml b/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml index faa7273dcd06..9a8657777a80 100644 --- a/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml +++ b/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml @@ -794,11 +794,22 @@ spec: will be disabled. If EnabledIf is not set, the patch will be enabled per default. type: string + external: + description: External defines an external patch. + properties: + generateExtension: + description: GenerateExtension references an extension which + is called to generate patches. + type: string + validateExtension: + description: ValidateExtension references an extension which + is called to validate the topology. + type: string + type: object name: description: Name of the patch. type: string required: - - definitions - name type: object type: array diff --git a/internal/controllers/topology/cluster/cluster_controller.go b/internal/controllers/topology/cluster/cluster_controller.go index b5d14e6fd575..a40862b12106 100644 --- a/internal/controllers/topology/cluster/cluster_controller.go +++ b/internal/controllers/topology/cluster/cluster_controller.go @@ -111,7 +111,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt r.externalTracker = external.ObjectTracker{ Controller: c, } - r.patchEngine = patches.NewEngine() + r.patchEngine = patches.NewEngine(r.RuntimeClient) r.recorder = mgr.GetEventRecorderFor("topology/cluster") if r.patchHelperFactory == nil { r.patchHelperFactory = serverSideApplyPatchHelperFactory(r.Client) @@ -121,7 +121,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt // SetupForDryRun prepares the Reconciler for a dry run execution. func (r *Reconciler) SetupForDryRun(recorder record.EventRecorder) { - r.patchEngine = patches.NewEngine() + r.patchEngine = patches.NewEngine(r.RuntimeClient) r.recorder = recorder r.patchHelperFactory = dryRunPatchHelperFactory(r.Client) } diff --git a/internal/controllers/topology/cluster/patches/api/interface.go b/internal/controllers/topology/cluster/patches/api/interface.go index 5fd5aef3b091..b1cbfba42a76 100644 --- a/internal/controllers/topology/cluster/patches/api/interface.go +++ b/internal/controllers/topology/cluster/patches/api/interface.go @@ -24,6 +24,8 @@ package api import ( "context" + "sigs.k8s.io/controller-runtime/pkg/client" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" ) @@ -32,5 +34,13 @@ type Generator interface { // Generate generates patches for templates. // 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 + Generate(context.Context, client.Object, *runtimehooksv1.GeneratePatchesRequest) (*runtimehooksv1.GeneratePatchesResponse, error) +} + +// Validator defines a component that can validate ClusterClass templates. +type Validator interface { + // Validate validates templates.. + // ValidateTopologyRequest contains templates and the corresponding variables. + // ValidateTopologyResponse contains the validation response. + Validate(context.Context, client.Object, *runtimehooksv1.ValidateTopologyRequest) (*runtimehooksv1.ValidateTopologyResponse, error) } diff --git a/internal/controllers/topology/cluster/patches/engine.go b/internal/controllers/topology/cluster/patches/engine.go index cfb6066eaf1e..8c4f85e828a6 100644 --- a/internal/controllers/topology/cluster/patches/engine.go +++ b/internal/controllers/topology/cluster/patches/engine.go @@ -26,12 +26,15 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" + "sigs.k8s.io/cluster-api/feature" "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/external" "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/inline" "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/variables" "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/scope" tlog "sigs.k8s.io/cluster-api/internal/log" + runtimeclient "sigs.k8s.io/cluster-api/internal/runtime/client" ) // Engine is a patch engine which applies patches defined in a ClusterBlueprint to a ClusterState. @@ -40,18 +43,15 @@ type Engine interface { } // NewEngine creates a new patch engine. -func NewEngine() Engine { +func NewEngine(runtimeClient runtimeclient.Client) Engine { return &engine{ - createPatchGenerator: createPatchGenerator, + runtimeClient: runtimeClient, } } // engine implements the Engine interface. type engine struct { - // createPatchGenerator is the func which returns a patch generator - // based on a ClusterClassPatch. - // Note: This field is also used to inject patches in unit tests. - createPatchGenerator func(patch *clusterv1.ClusterClassPatch) (api.Generator, error) + runtimeClient runtimeclient.Client } // Apply applies patches to the desired state according to the patches from the ClusterClass, variables from the Cluster @@ -83,7 +83,7 @@ func (e *engine) Apply(ctx context.Context, blueprint *scope.ClusterBlueprint, d log.V(5).Infof("Applying patch to templates") // Create patch generator for the current patch. - generator, err := e.createPatchGenerator(&clusterClassPatch) + generator, err := createPatchGenerator(e.runtimeClient, &clusterClassPatch) if err != nil { return err } @@ -92,9 +92,9 @@ 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 := generator.Generate(ctx, req) - if resp.Status == runtimehooksv1.ResponseStatusFailure { - return errors.Errorf("failed to generate patches for patch %q: %v", clusterClassPatch.Name, resp.Message) + resp, err := generator.Generate(ctx, desired.Cluster, req) + if err != nil { + return errors.Wrapf(err, "failed to generate patches for patch %q", clusterClassPatch.Name) } // Apply patches to the request. @@ -103,6 +103,30 @@ func (e *engine) Apply(ctx context.Context, blueprint *scope.ClusterBlueprint, d } } + // Convert request to validation request. + validationRequest := convertToValidationRequest(req) + + // Loop over patches in ClusterClass and validate topology, + // respecting the order in which they are defined. + for i := range blueprint.ClusterClass.Spec.Patches { + clusterClassPatch := blueprint.ClusterClass.Spec.Patches[i] + + if clusterClassPatch.External == nil || clusterClassPatch.External.ValidateExtension == nil { + continue + } + + ctx, log = log.WithValues("patch", clusterClassPatch.Name).Into(ctx) + + log.V(5).Infof("Validating topology") + + validator := external.NewValidator(e.runtimeClient, &clusterClassPatch) + + _, err := validator.Validate(ctx, desired.Cluster, validationRequest) + if err != nil { + return errors.Wrapf(err, "validation of patch %q failed", clusterClassPatch.Name) + } + } + // Use patched templates to update the desired state objects. log.V(5).Infof("Applying patched templates to desired state") if err := updateDesiredState(ctx, req, blueprint, desired); err != nil { @@ -234,10 +258,20 @@ func lookupMDTopology(topology *clusterv1.Topology, mdTopologyName string) (*clu // createPatchGenerator creates a patch generator for the given patch. // NOTE: Currently only inline JSON patches are supported; in the future we will add // external patches as well. -func createPatchGenerator(patch *clusterv1.ClusterClassPatch) (api.Generator, error) { +func createPatchGenerator(runtimeClient runtimeclient.Client, patch *clusterv1.ClusterClassPatch) (api.Generator, error) { // Return a jsonPatchGenerator if there are PatchDefinitions in the patch. if len(patch.Definitions) > 0 { - return inline.New(patch), nil + return inline.NewGenerator(patch), nil + } + // Return an externalPatchGenerator if there is an external configuration in the patch. + if patch.External != nil && patch.External.GenerateExtension != nil { + if !feature.Gates.Enabled(feature.RuntimeSDK) { + return nil, errors.Errorf("can not use external patch %q if RuntimeSDK feature flag is disabled", patch.Name) + } + if runtimeClient == nil { + return nil, errors.Errorf("failed to create patch generator for patch %q: runtimeClient is not set up", patch.Name) + } + return external.NewGenerator(runtimeClient, patch), nil } return nil, errors.Errorf("failed to create patch generator for patch %q", patch.Name) @@ -296,6 +330,24 @@ func applyPatchesToRequest(ctx context.Context, req *runtimehooksv1.GeneratePatc return nil } +// convertToValidationRequest converts a GeneratePatchesRequest to a ValidateTopologyRequest. +func convertToValidationRequest(generateRequest *runtimehooksv1.GeneratePatchesRequest) *runtimehooksv1.ValidateTopologyRequest { + validationRequest := &runtimehooksv1.ValidateTopologyRequest{} + validationRequest.Variables = generateRequest.Variables + + for i := range generateRequest.Items { + item := generateRequest.Items[i] + + validationRequest.Items = append(validationRequest.Items, &runtimehooksv1.ValidateTopologyRequestItem{ + HolderReference: item.HolderReference, + Object: item.Object, + Variables: item.Variables, + }) + } + + return validationRequest +} + // 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 { diff --git a/internal/controllers/topology/cluster/patches/engine_test.go b/internal/controllers/topology/cluster/patches/engine_test.go index f64d0f7204bf..5538debc16a3 100644 --- a/internal/controllers/topology/cluster/patches/engine_test.go +++ b/internal/controllers/topology/cluster/patches/engine_test.go @@ -29,7 +29,12 @@ import ( "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + runtimev1 "sigs.k8s.io/cluster-api/exp/runtime/api/v1alpha1" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/scope" + runtimecatalog "sigs.k8s.io/cluster-api/internal/runtime/catalog" + runtimeclient "sigs.k8s.io/cluster-api/internal/runtime/client" + runtimeregistry "sigs.k8s.io/cluster-api/internal/runtime/registry" "sigs.k8s.io/cluster-api/internal/test/builder" . "sigs.k8s.io/cluster-api/internal/test/matchers" ) @@ -361,7 +366,17 @@ func TestApply(t *testing.T) { blueprint, desired := setupTestObjects() // If there are patches, set up patch generators. - patchEngine := NewEngine() + // FIXME(sbueringer) implement test for external patches + cat := runtimecatalog.New() + g.Expect(runtimehooksv1.AddToCatalog(cat)).To(Succeed()) + + registry := runtimeregistry.New() + g.Expect(registry.WarmUp(&runtimev1.ExtensionConfigList{})).To(Succeed()) + runtimeClient := runtimeclient.New(runtimeclient.Options{ + Catalog: cat, + Registry: registry, + }) + patchEngine := NewEngine(runtimeClient) if len(tt.patches) > 0 { // Add the patches. blueprint.ClusterClass.Spec.Patches = tt.patches diff --git a/internal/controllers/topology/cluster/patches/external/external_patch_generator.go b/internal/controllers/topology/cluster/patches/external/external_patch_generator.go new file mode 100644 index 000000000000..7da6cef2c8fb --- /dev/null +++ b/internal/controllers/topology/cluster/patches/external/external_patch_generator.go @@ -0,0 +1,57 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package external implements the external patch generator. +package external + +import ( + "context" + + "github.com/pkg/errors" + "sigs.k8s.io/controller-runtime/pkg/client" + + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" + "sigs.k8s.io/cluster-api/feature" + "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/api" + runtimeclient "sigs.k8s.io/cluster-api/internal/runtime/client" +) + +// externalPatchGenerator generates JSON patches for a GeneratePatchesRequest based on a ClusterClassPatch. +type externalPatchGenerator struct { + runtimeClient runtimeclient.Client + patch *clusterv1.ClusterClassPatch +} + +// NewGenerator returns a new external Generator from a given ClusterClassPatch object. +func NewGenerator(runtimeClient runtimeclient.Client, patch *clusterv1.ClusterClassPatch) api.Generator { + return &externalPatchGenerator{ + runtimeClient: runtimeClient, + patch: patch, + } +} + +func (e externalPatchGenerator) Generate(ctx context.Context, forObject client.Object, req *runtimehooksv1.GeneratePatchesRequest) (*runtimehooksv1.GeneratePatchesResponse, error) { + if !feature.Gates.Enabled(feature.RuntimeSDK) { + return nil, errors.Errorf("can not use external patch %q if RuntimeSDK feature flag is disabled", *e.patch.External.GenerateExtension) + } + resp := &runtimehooksv1.GeneratePatchesResponse{} + err := e.runtimeClient.CallExtension(ctx, runtimehooksv1.GeneratePatches, forObject, *e.patch.External.GenerateExtension, req, resp) + if err != nil { + return nil, err + } + return resp, nil +} diff --git a/internal/controllers/topology/cluster/patches/external/external_validator.go b/internal/controllers/topology/cluster/patches/external/external_validator.go new file mode 100644 index 000000000000..c238de0503e9 --- /dev/null +++ b/internal/controllers/topology/cluster/patches/external/external_validator.go @@ -0,0 +1,57 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package external implements the external patch generator. +package external + +import ( + "context" + + "github.com/pkg/errors" + "sigs.k8s.io/controller-runtime/pkg/client" + + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" + "sigs.k8s.io/cluster-api/feature" + "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/api" + runtimeclient "sigs.k8s.io/cluster-api/internal/runtime/client" +) + +// externalValidator validates templates. +type externalValidator struct { + runtimeClient runtimeclient.Client + patch *clusterv1.ClusterClassPatch +} + +// NewValidator returns a new external Validator from a given ClusterClassPatch object. +func NewValidator(runtimeClient runtimeclient.Client, patch *clusterv1.ClusterClassPatch) api.Validator { + return &externalValidator{ + runtimeClient: runtimeClient, + patch: patch, + } +} + +func (e externalValidator) Validate(ctx context.Context, forObject client.Object, req *runtimehooksv1.ValidateTopologyRequest) (*runtimehooksv1.ValidateTopologyResponse, error) { + if !feature.Gates.Enabled(feature.RuntimeSDK) { + return nil, errors.Errorf("can not use external patch %q if RuntimeSDK feature flag is disabled", *e.patch.External.ValidateExtension) + } + resp := &runtimehooksv1.ValidateTopologyResponse{} + err := e.runtimeClient.CallExtension(ctx, runtimehooksv1.ValidateTopology, forObject, *e.patch.External.ValidateExtension, req, resp) + if err != nil { + return nil, err + } + return resp, nil +} 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 55dfc45687f9..71decbea10de 100644 --- a/internal/controllers/topology/cluster/patches/inline/json_patch_generator.go +++ b/internal/controllers/topology/cluster/patches/inline/json_patch_generator.go @@ -29,6 +29,7 @@ import ( "github.com/pkg/errors" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" kerrors "k8s.io/apimachinery/pkg/util/errors" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -43,15 +44,15 @@ type jsonPatchGenerator struct { patch *clusterv1.ClusterClassPatch } -// New returns a new inline Generator from a given ClusterClassPatch object. -func New(patch *clusterv1.ClusterClassPatch) api.Generator { +// NewGenerator returns a new inline Generator from a given ClusterClassPatch object. +func NewGenerator(patch *clusterv1.ClusterClassPatch) api.Generator { return &jsonPatchGenerator{ patch: patch, } } // Generate generates JSON patches for the given GeneratePatchesRequest based on a ClusterClassPatch. -func (j *jsonPatchGenerator) Generate(_ context.Context, req *runtimehooksv1.GeneratePatchesRequest) *runtimehooksv1.GeneratePatchesResponse { +func (j *jsonPatchGenerator) Generate(_ context.Context, _ client.Object, req *runtimehooksv1.GeneratePatchesRequest) (*runtimehooksv1.GeneratePatchesResponse, error) { resp := &runtimehooksv1.GeneratePatchesResponse{} globalVariables := patchvariables.ToMap(req.Variables) @@ -113,15 +114,10 @@ func (j *jsonPatchGenerator) Generate(_ context.Context, req *runtimehooksv1.Gen } if err := kerrors.NewAggregate(errs); err != nil { - return &runtimehooksv1.GeneratePatchesResponse{ - CommonResponse: runtimehooksv1.CommonResponse{ - Status: runtimehooksv1.ResponseStatusFailure, - Message: err.Error(), - }, - } + return nil, err } - return resp + return resp, nil } // matchesSelector returns true if the GeneratePatchesRequestItem matches the selector. 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 b9e169f5436a..9aa7f2f0444b 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 @@ -24,6 +24,7 @@ import ( . "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/apimachinery/pkg/runtime" "k8s.io/utils/pointer" @@ -339,9 +340,10 @@ func TestGenerate(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - got := New(tt.patch).Generate(context.Background(), tt.req) + got, err := NewGenerator(tt.patch).Generate(context.Background(), &clusterv1.Cluster{ObjectMeta: metav1.ObjectMeta{Namespace: "default"}}, tt.req) g.Expect(got).To(Equal(tt.want)) + g.Expect(err).ToNot(HaveOccurred()) }) } } diff --git a/internal/webhooks/patch_validation.go b/internal/webhooks/patch_validation.go index 8b2544f2f981..fc3238d437d8 100644 --- a/internal/webhooks/patch_validation.go +++ b/internal/webhooks/patch_validation.go @@ -30,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/feature" ) // validatePatches returns errors if the Patches in the ClusterClass violate any validation rules. @@ -85,11 +86,47 @@ func validatePatchDefinitions(patch clusterv1.ClusterClassPatch, clusterClass *c allErrs = append(allErrs, validateEnabledIf(patch.EnabledIf, path.Child("enabledIf"))...) - for i, definition := range patch.Definitions { + if patch.Definitions == nil && patch.External == nil { allErrs = append(allErrs, - validateJSONPatches(definition.JSONPatches, clusterClass.Spec.Variables, path.Child("definitions").Index(i).Child("jsonPatches"))...) + field.Required( + path, + "one of definitions or external must be defined", + )) + } + + if patch.Definitions != nil && patch.External != nil { allErrs = append(allErrs, - validateSelectors(definition.Selector, clusterClass, path.Child("definitions").Index(i).Child("selector"))...) + field.Invalid( + path, + patch, + "only one of definitions or external can be defined", + )) + } + + if patch.Definitions != nil { + for i, definition := range patch.Definitions { + allErrs = append(allErrs, + validateJSONPatches(definition.JSONPatches, clusterClass.Spec.Variables, path.Child("definitions").Index(i).Child("jsonPatches"))...) + allErrs = append(allErrs, + validateSelectors(definition.Selector, clusterClass, path.Child("definitions").Index(i).Child("selector"))...) + } + } + if patch.External != nil { + if !feature.Gates.Enabled(feature.RuntimeSDK) { + allErrs = append(allErrs, + field.Forbidden( + path.Child("external"), + "patch.external can be used only if the RuntimeSDK feature flag is enabled", + )) + } + if patch.External.ValidateExtension == nil && patch.External.GenerateExtension == nil { + allErrs = append(allErrs, + field.Invalid( + path.Child("external"), + patch.External, + "one of validateExtension and generateExtension must be defined", + )) + } } return allErrs } diff --git a/internal/webhooks/patch_validation_test.go b/internal/webhooks/patch_validation_test.go index e54cf421a79f..376290c5733a 100644 --- a/internal/webhooks/patch_validation_test.go +++ b/internal/webhooks/patch_validation_test.go @@ -24,9 +24,11 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" + utilfeature "k8s.io/component-base/featuregate/testing" "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/test/builder" ) @@ -34,6 +36,7 @@ func TestValidatePatches(t *testing.T) { tests := []struct { name string clusterClass clusterv1.ClusterClass + runtimeSDK bool wantErr bool }{ { @@ -279,8 +282,9 @@ func TestValidatePatches(t *testing.T) { }, Patches: []clusterv1.ClusterClassPatch{ { - Name: "patch1", - EnabledIf: pointer.String(`template {{ .variableB }}`), + Name: "patch1", + EnabledIf: pointer.String(`template {{ .variableB }}`), + Definitions: []clusterv1.PatchDefinition{}, }, }, }, @@ -1378,9 +1382,142 @@ func TestValidatePatches(t *testing.T) { }, wantErr: false, }, + + // Patch with External + { + name: "pass if patch defines both external.generateExtension and external.validateExtension", + clusterClass: clusterv1.ClusterClass{ + Spec: clusterv1.ClusterClassSpec{ + ControlPlane: clusterv1.ControlPlaneClass{ + LocalObjectTemplate: clusterv1.LocalObjectTemplate{ + Ref: &corev1.ObjectReference{ + APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", + Kind: "ControlPlaneTemplate", + }, + }, + }, + + Patches: []clusterv1.ClusterClassPatch{ + { + Name: "patch1", + External: &clusterv1.ExternalPatchDefinition{ + GenerateExtension: pointer.String("generate-extension"), + ValidateExtension: pointer.String("generate-extension"), + }, + }, + }, + }, + }, + runtimeSDK: true, + wantErr: false, + }, + { + name: "error if patch defines both external and RuntimeSDK is not enabled", + clusterClass: clusterv1.ClusterClass{ + Spec: clusterv1.ClusterClassSpec{ + ControlPlane: clusterv1.ControlPlaneClass{ + LocalObjectTemplate: clusterv1.LocalObjectTemplate{ + Ref: &corev1.ObjectReference{ + APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", + Kind: "ControlPlaneTemplate", + }, + }, + }, + + Patches: []clusterv1.ClusterClassPatch{ + { + Name: "patch1", + External: &clusterv1.ExternalPatchDefinition{ + GenerateExtension: pointer.String("generate-extension"), + ValidateExtension: pointer.String("generate-extension"), + }, + }, + }, + }, + }, + runtimeSDK: false, + wantErr: true, + }, + { + name: "error if patch defines neither external.generateExtension nor external.validateExtension", + clusterClass: clusterv1.ClusterClass{ + Spec: clusterv1.ClusterClassSpec{ + ControlPlane: clusterv1.ControlPlaneClass{ + LocalObjectTemplate: clusterv1.LocalObjectTemplate{ + Ref: &corev1.ObjectReference{ + APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", + Kind: "ControlPlaneTemplate", + }, + }, + }, + + Patches: []clusterv1.ClusterClassPatch{ + { + Name: "patch1", + External: &clusterv1.ExternalPatchDefinition{}, + }, + }, + }, + }, + runtimeSDK: true, + wantErr: true, + }, + { + name: "error if patch defines both external and definitions", + clusterClass: clusterv1.ClusterClass{ + Spec: clusterv1.ClusterClassSpec{ + ControlPlane: clusterv1.ControlPlaneClass{ + LocalObjectTemplate: clusterv1.LocalObjectTemplate{ + Ref: &corev1.ObjectReference{ + APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", + Kind: "ControlPlaneTemplate", + }, + }, + }, + + Patches: []clusterv1.ClusterClassPatch{ + { + Name: "patch1", + External: &clusterv1.ExternalPatchDefinition{ + GenerateExtension: pointer.String("generate-extension"), + ValidateExtension: pointer.String("generate-extension"), + }, + Definitions: []clusterv1.PatchDefinition{}, + }, + }, + }, + }, + runtimeSDK: true, + wantErr: true, + }, + { + name: "error if neither external nor definitions is defined", + clusterClass: clusterv1.ClusterClass{ + Spec: clusterv1.ClusterClassSpec{ + ControlPlane: clusterv1.ControlPlaneClass{ + LocalObjectTemplate: clusterv1.LocalObjectTemplate{ + Ref: &corev1.ObjectReference{ + APIVersion: "controlplane.cluster.x-k8s.io/v1beta1", + Kind: "ControlPlaneTemplate", + }, + }, + }, + + Patches: []clusterv1.ClusterClassPatch{ + { + Name: "patch1", + }, + }, + }, + }, + runtimeSDK: true, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.RuntimeSDK, tt.runtimeSDK)() + g := NewWithT(t) errList := validatePatches(&tt.clusterClass) diff --git a/test/e2e/data/infrastructure-docker/v1beta1/clusterclass-quick-start-runtimesdk.yaml b/test/e2e/data/infrastructure-docker/v1beta1/clusterclass-quick-start-runtimesdk.yaml index 054631192f74..d880f4cfa10c 100644 --- a/test/e2e/data/infrastructure-docker/v1beta1/clusterclass-quick-start-runtimesdk.yaml +++ b/test/e2e/data/infrastructure-docker/v1beta1/clusterclass-quick-start-runtimesdk.yaml @@ -72,11 +72,10 @@ spec: example: "0" description: "kubeadmControlPlaneMaxSurge is the maximum number of control planes that can be scheduled above or under the desired number of control plane machines." patches: -# TODO: enable external patches once topology mutation is implemented -# - name: lbImageRepository -# external: -# generateExtension: generate-patches.k8s-upgrade-with-runtimesdk -# validateExtension: validate-topology.k8s-upgrade-with-runtimesdk + - name: lbImageRepository + external: + generateExtension: generate-patches.k8s-upgrade-with-runtimesdk + validateExtension: validate-topology.k8s-upgrade-with-runtimesdk - name: imageRepository description: "Sets the imageRepository used for the KubeadmControlPlane." definitions: diff --git a/test/extension/server/server.go b/test/extension/server/server.go index 7e0f0d4791b7..3c4c5e397ea3 100644 --- a/test/extension/server/server.go +++ b/test/extension/server/server.go @@ -230,20 +230,22 @@ func (s *Server) Start(ctx context.Context) error { // discoveryHandler generates a discovery handler based on a list of handlers. func discoveryHandler(handlers map[string]ExtensionHandler) func(context.Context, *runtimehooksv1.DiscoveryRequest, *runtimehooksv1.DiscoveryResponse) { - return func(_ context.Context, request *runtimehooksv1.DiscoveryRequest, response *runtimehooksv1.DiscoveryResponse) { - response.Status = runtimehooksv1.ResponseStatusSuccess - - for _, handler := range handlers { - response.Handlers = append(response.Handlers, runtimehooksv1.ExtensionHandler{ - Name: handler.Name, - RequestHook: runtimehooksv1.GroupVersionHook{ - APIVersion: handler.gvh.GroupVersion().String(), - Hook: handler.gvh.Hook, - }, - TimeoutSeconds: handler.TimeoutSeconds, - FailurePolicy: handler.FailurePolicy, - }) - } + cachedHandlers := []runtimehooksv1.ExtensionHandler{} + for _, handler := range handlers { + cachedHandlers = append(cachedHandlers, runtimehooksv1.ExtensionHandler{ + Name: handler.Name, + RequestHook: runtimehooksv1.GroupVersionHook{ + APIVersion: handler.gvh.GroupVersion().String(), + Hook: handler.gvh.Hook, + }, + TimeoutSeconds: handler.TimeoutSeconds, + FailurePolicy: handler.FailurePolicy, + }) + } + + return func(_ context.Context, _ *runtimehooksv1.DiscoveryRequest, response *runtimehooksv1.DiscoveryResponse) { + response.SetStatus(runtimehooksv1.ResponseStatusSuccess) + response.Handlers = cachedHandlers } }