Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Topology Mutation Hook: Implement external patching #6623

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion api/v1beta1/clusterclass_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
30 changes: 30 additions & 0 deletions api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 36 additions & 2 deletions api/v1beta1/zz_generated.openapi.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 12 additions & 1 deletion config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions internal/controllers/topology/cluster/cluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down
12 changes: 11 additions & 1 deletion internal/controllers/topology/cluster/patches/api/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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)
}
76 changes: 64 additions & 12 deletions internal/controllers/topology/cluster/patches/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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.
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
17 changes: 16 additions & 1 deletion internal/controllers/topology/cluster/patches/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current version is still missing these tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify: Plan is to add them in a follow-up PR which will be up today or tomorrow

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
Expand Down
Loading