diff --git a/api/v1beta1/clusterclass_types.go b/api/v1beta1/clusterclass_types.go index a2dcab4c9cec..0af9525f248c 100644 --- a/api/v1beta1/clusterclass_types.go +++ b/api/v1beta1/clusterclass_types.go @@ -342,7 +342,7 @@ type ClusterClassPatch struct { Definitions []PatchDefinition `json:"definitions,omitempty"` // External defines an external patch. - // FIXME(sbueringer): internal or external only => validate + // FIXME(sbueringer): implement validation that exactly one of Definitions/External is set // +optional External *ExternalPatchDefinition `json:"external,omitempty"` } @@ -449,8 +449,9 @@ type JSONPatchValue struct { // ExternalPatchDefinition defines an external patch. type ExternalPatchDefinition struct { + // FIXME(sbueringer): implement validation that at least one of them is set if parent exists // GenerateExtension references an extension which is called to generate patches. - // FIXME(sbueringer): Q: Fine that GenerateExtension is optional as well? + // TBD here: https://vmware.slack.com/archives/C02TJ7SNQU8/p1654863214046059 // +optional GenerateExtension *string `json:"generateExtension,omitempty"` diff --git a/api/v1beta1/zz_generated.openapi.go b/api/v1beta1/zz_generated.openapi.go index 7cd3969b28c9..4c699ea57527 100644 --- a/api/v1beta1/zz_generated.openapi.go +++ b/api/v1beta1/zz_generated.openapi.go @@ -331,7 +331,7 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_ClusterClassPatch(ref common.Refer }, "external": { SchemaProps: spec.SchemaProps{ - Description: "External defines an external patch. FIXME(sbueringer): internal or external only => validate", + Description: "External defines an external patch. FIXME(sbueringer): implement validation that exactly one of Definitions/External is set", Ref: ref("sigs.k8s.io/cluster-api/api/v1beta1.ExternalPatchDefinition"), }, }, @@ -854,7 +854,7 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_ExternalPatchDefinition(ref common Properties: map[string]spec.Schema{ "generateExtension": { SchemaProps: spec.SchemaProps{ - Description: "GenerateExtension references an extension which is called to generate patches. FIXME(sbueringer): Q: Fine that GenerateExtension is optional as well?", + Description: "FIXME(sbueringer): implement validation that at least one of them is set if parent exists GenerateExtension references an extension which is called to generate patches. TBD here: https://vmware.slack.com/archives/C02TJ7SNQU8/p1654863214046059", Type: []string{"string"}, Format: "", }, diff --git a/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml b/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml index bac03994c247..66b353855629 100644 --- a/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml +++ b/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml @@ -796,12 +796,14 @@ spec: type: string external: description: 'External defines an external patch. FIXME(sbueringer): - internal or external only => validate' + implement validation that exactly one of Definitions/External + is set' properties: generateExtension: - description: 'GenerateExtension references an extension - which is called to generate patches. FIXME(sbueringer): - Q: Fine that GenerateExtension is optional as well?' + description: 'FIXME(sbueringer): implement validation that + at least one of them is set if parent exists GenerateExtension + references an extension which is called to generate patches. + TBD here: https://vmware.slack.com/archives/C02TJ7SNQU8/p1654863214046059' type: string validateExtension: description: GenerateExtension references an extension which diff --git a/controllers/alias.go b/controllers/alias.go index 077955502738..c56d8af45f40 100644 --- a/controllers/alias.go +++ b/controllers/alias.go @@ -150,7 +150,6 @@ func (r *ClusterTopologyReconciler) SetupWithManager(ctx context.Context, mgr ct APIReader: r.APIReader, RuntimeClient: r.RuntimeClient, UnstructuredCachingClient: r.UnstructuredCachingClient, - RuntimeClient: r.RuntimeClient, WatchFilterValue: r.WatchFilterValue, }).SetupWithManager(ctx, mgr, options) } diff --git a/internal/controllers/topology/cluster/cluster_controller.go b/internal/controllers/topology/cluster/cluster_controller.go index c697afa1ec5e..a40862b12106 100644 --- a/internal/controllers/topology/cluster/cluster_controller.go +++ b/internal/controllers/topology/cluster/cluster_controller.go @@ -121,7 +121,6 @@ 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) { - // FIXME(sbueringer) this won't work without a real registry. r.patchEngine = patches.NewEngine(r.RuntimeClient) r.recorder = recorder r.patchHelperFactory = dryRunPatchHelperFactory(r.Client) diff --git a/internal/controllers/topology/cluster/patches/engine.go b/internal/controllers/topology/cluster/patches/engine.go index b20885719d11..9031ed729dc8 100644 --- a/internal/controllers/topology/cluster/patches/engine.go +++ b/internal/controllers/topology/cluster/patches/engine.go @@ -82,17 +82,11 @@ 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 := createPatchGenerator(e.runtimeClient, &clusterClassPatch) + generator, err := createPatchGenerator(e.runtimeClient, desired.Cluster, &clusterClassPatch) if err != nil { return err } - // Continue if no generator has been created. This can happen if an external config - // does not contain a generate extension. - if generator == nil { - continue - } - // Generate patches. // 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 @@ -124,11 +118,11 @@ func (e *engine) Apply(ctx context.Context, blueprint *scope.ClusterBlueprint, d log.V(5).Infof("Validating topology") - validator := external.NewValidator(e.runtimeClient, &clusterClassPatch) + validator := external.NewValidator(e.runtimeClient, desired.Cluster, &clusterClassPatch) _, err := validator.Validate(ctx, validationRequest) if err != nil { - return errors.Wrapf(err, "template validation %q failed", clusterClassPatch.Name) + return errors.Wrapf(err, "validation of patch %q failed", clusterClassPatch.Name) } } @@ -263,17 +257,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(runtimeClient runtimeclient.Client, patch *clusterv1.ClusterClassPatch) (api.Generator, error) { +func createPatchGenerator(runtimeClient runtimeclient.Client, cluster *clusterv1.Cluster, 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 an externalPatchGenerator if there is an external configuration in the patch. - if patch.External != nil { - if patch.External.GenerateExtension == nil { - return nil, nil + if patch.External != nil && patch.External.GenerateExtension != nil { + // FIXME(sbueringer): let's check for the RuntimeSDK feature flag: + // 1. in the validating webhook + // 2. here + if runtimeClient == nil { + return nil, errors.Errorf("failed to create patch generator for patch %q: runtimeClient is not set up", patch.Name) } - return external.New(runtimeClient, patch), nil + return external.New(runtimeClient, cluster, patch), nil } return nil, errors.Errorf("failed to create patch generator for patch %q", patch.Name) diff --git a/internal/controllers/topology/cluster/patches/external/external_patch_generator.go b/internal/controllers/topology/cluster/patches/external/external_patch_generator.go index ad58c35d577f..6c786c38266b 100644 --- a/internal/controllers/topology/cluster/patches/external/external_patch_generator.go +++ b/internal/controllers/topology/cluster/patches/external/external_patch_generator.go @@ -30,20 +30,21 @@ import ( type externalPatchGenerator struct { runtimeClient runtimeclient.Client patch *clusterv1.ClusterClassPatch + cluster *clusterv1.Cluster } // New returns a new external Generator from a given ClusterClassPatch object. -func New(runtimeClient runtimeclient.Client, patch *clusterv1.ClusterClassPatch) api.Generator { +func New(runtimeClient runtimeclient.Client, cluster *clusterv1.Cluster, patch *clusterv1.ClusterClassPatch) api.Generator { return &externalPatchGenerator{ runtimeClient: runtimeClient, + cluster: cluster, patch: patch, } } func (e externalPatchGenerator) Generate(ctx context.Context, req *runtimehooksv1.GeneratePatchesRequest) (*runtimehooksv1.GeneratePatchesResponse, error) { resp := &runtimehooksv1.GeneratePatchesResponse{} - // FIXME(sbueringer): CallExtension and CallAllExtensions should check if registry is ready internally. - err := e.runtimeClient.CallExtension(ctx, runtimehooksv1.GeneratePatches, *e.patch.External.GenerateExtension, req, resp) + err := e.runtimeClient.CallExtension(ctx, runtimehooksv1.GeneratePatches, e.cluster, *e.patch.External.GenerateExtension, req, resp) if err != nil { return nil, err } diff --git a/internal/controllers/topology/cluster/patches/external/external_validator.go b/internal/controllers/topology/cluster/patches/external/external_validator.go index e3866128c764..73bbb6933a07 100644 --- a/internal/controllers/topology/cluster/patches/external/external_validator.go +++ b/internal/controllers/topology/cluster/patches/external/external_validator.go @@ -30,19 +30,21 @@ import ( type externalValidator struct { runtimeClient runtimeclient.Client patch *clusterv1.ClusterClassPatch + cluster *clusterv1.Cluster } // NewValidator returns a new external Validator from a given ClusterClassPatch object. -func NewValidator(runtimeClient runtimeclient.Client, patch *clusterv1.ClusterClassPatch) api.Validator { +func NewValidator(runtimeClient runtimeclient.Client, cluster *clusterv1.Cluster, patch *clusterv1.ClusterClassPatch) api.Validator { return &externalValidator{ runtimeClient: runtimeClient, + cluster: cluster, patch: patch, } } func (e externalValidator) Validate(ctx context.Context, req *runtimehooksv1.ValidateTopologyRequest) (*runtimehooksv1.ValidateTopologyResponse, error) { resp := &runtimehooksv1.ValidateTopologyResponse{} - err := e.runtimeClient.CallExtension(ctx, runtimehooksv1.ValidateTopology, *e.patch.External.ValidateExtension, req, resp) + err := e.runtimeClient.CallExtension(ctx, runtimehooksv1.ValidateTopology, e.cluster, *e.patch.External.ValidateExtension, req, resp) if err != nil { return nil, err } diff --git a/poc/local/README.md b/poc/local/README.md index 7aa36c2a77b0..9a5622e59d17 100644 --- a/poc/local/README.md +++ b/poc/local/README.md @@ -29,7 +29,7 @@ k apply -f ./poc/test/local/secure-infra.yaml # fetch certificate mkdir -p /tmp/k8s-webhook-server/serving-certs -for f in $(kubectl get secret my-local-extension-cert -o json | jq '.data | keys | .[]' -r); do +for f in $(kubectl get secret webhook-service-cert -o json | jq '.data | keys | .[]' -r); do kubectl get secret my-local-extension-cert -o json | jq '.data["'$f'"]' -r | base64 -d > "/tmp/k8s-webhook-server/serving-certs/$f" done