From 1f0b3194bca51591569c1685ac4507742e774d0e Mon Sep 17 00:00:00 2001 From: alexander-demicev Date: Thu, 12 Oct 2023 20:29:30 +0200 Subject: [PATCH 1/5] Add resource patches to API --- api/v1alpha1/provider_conversion.go | 101 ++++++++++++++++-- api/v1alpha1/zz_generated.conversion.go | 1 + api/v1alpha2/provider_types.go | 8 ++ api/v1alpha2/zz_generated.deepcopy.go | 5 + ...rator.cluster.x-k8s.io_addonproviders.yaml | 9 ++ ...r.cluster.x-k8s.io_bootstrapproviders.yaml | 9 ++ ...luster.x-k8s.io_controlplaneproviders.yaml | 9 ++ ...erator.cluster.x-k8s.io_coreproviders.yaml | 9 ++ ...ster.x-k8s.io_infrastructureproviders.yaml | 9 ++ 9 files changed, 152 insertions(+), 8 deletions(-) diff --git a/api/v1alpha1/provider_conversion.go b/api/v1alpha1/provider_conversion.go index 9666b8a0..05e06ee4 100644 --- a/api/v1alpha1/provider_conversion.go +++ b/api/v1alpha1/provider_conversion.go @@ -21,6 +21,7 @@ import ( apimachineryconversion "k8s.io/apimachinery/pkg/conversion" "k8s.io/utils/pointer" + utilconversion "sigs.k8s.io/cluster-api/util/conversion" ctrlconfigv1 "sigs.k8s.io/controller-runtime/pkg/config/v1alpha1" "sigs.k8s.io/controller-runtime/pkg/conversion" @@ -34,7 +35,19 @@ func (src *BootstrapProvider) ConvertTo(dstRaw conversion.Hub) error { panic("expected to get an of object of type v1alpha2.BootstrapProvider") } - return Convert_v1alpha1_BootstrapProvider_To_v1alpha2_BootstrapProvider(src, dst, nil) + if err := Convert_v1alpha1_BootstrapProvider_To_v1alpha2_BootstrapProvider(src, dst, nil); err != nil { + return err + } + + // Manually restore data. + restored := &operatorv1.BootstrapProvider{} + if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok { + return err + } + + dst.Spec.ManifestPatches = restored.Spec.ManifestPatches + + return nil } // ConvertFrom converts from the BootstrapProvider version (v1alpha2) to this version. @@ -44,7 +57,16 @@ func (dst *BootstrapProvider) ConvertFrom(srcRaw conversion.Hub) error { panic("expected to get an of object of type v1alpha2.BootstrapProvider") } - return Convert_v1alpha2_BootstrapProvider_To_v1alpha1_BootstrapProvider(src, dst, nil) + if err := Convert_v1alpha2_BootstrapProvider_To_v1alpha1_BootstrapProvider(src, dst, nil); err != nil { + return err + } + + // Preserve Hub data on down-conversion. + if err := utilconversion.MarshalData(src, dst); err != nil { + return err + } + + return nil } // ConvertTo converts this BootstrapProviderList to the Hub version (v1alpha2). @@ -74,7 +96,19 @@ func (src *ControlPlaneProvider) ConvertTo(dstRaw conversion.Hub) error { panic("expected to get an of object of type v1alpha2.ControlPlaneProvider") } - return Convert_v1alpha1_ControlPlaneProvider_To_v1alpha2_ControlPlaneProvider(src, dst, nil) + if err := Convert_v1alpha1_ControlPlaneProvider_To_v1alpha2_ControlPlaneProvider(src, dst, nil); err != nil { + return err + } + + // Manually restore data. + restored := &operatorv1.ControlPlaneProvider{} + if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok { + return err + } + + dst.Spec.ManifestPatches = restored.Spec.ManifestPatches + + return nil } // ConvertFrom converts from the ControlPlaneProvider version (v1alpha2) to this version. @@ -84,7 +118,16 @@ func (dst *ControlPlaneProvider) ConvertFrom(srcRaw conversion.Hub) error { panic("expected to get an of object of type v1alpha2.ControlPlaneProvider") } - return Convert_v1alpha2_ControlPlaneProvider_To_v1alpha1_ControlPlaneProvider(src, dst, nil) + if err := Convert_v1alpha2_ControlPlaneProvider_To_v1alpha1_ControlPlaneProvider(src, dst, nil); err != nil { + return err + } + + // Preserve Hub data on down-conversion. + if err := utilconversion.MarshalData(src, dst); err != nil { + return err + } + + return nil } // ConvertTo converts this ControlPlaneProviderList to the Hub version (v1alpha2). @@ -114,7 +157,19 @@ func (src *CoreProvider) ConvertTo(dstRaw conversion.Hub) error { panic("expected to get an of object of type v1alpha2.CoreProvider") } - return Convert_v1alpha1_CoreProvider_To_v1alpha2_CoreProvider(src, dst, nil) + if err := Convert_v1alpha1_CoreProvider_To_v1alpha2_CoreProvider(src, dst, nil); err != nil { + return err + } + + // Manually restore data. + restored := &operatorv1.CoreProvider{} + if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok { + return err + } + + dst.Spec.ManifestPatches = restored.Spec.ManifestPatches + + return nil } // ConvertFrom converts from the CoreProvider version (v1alpha2) to this version. @@ -124,7 +179,16 @@ func (dst *CoreProvider) ConvertFrom(srcRaw conversion.Hub) error { panic("expected to get an of object of type v1alpha2.CoreProvider") } - return Convert_v1alpha2_CoreProvider_To_v1alpha1_CoreProvider(src, dst, nil) + if err := Convert_v1alpha2_CoreProvider_To_v1alpha1_CoreProvider(src, dst, nil); err != nil { + return err + } + + // Preserve Hub data on down-conversion. + if err := utilconversion.MarshalData(src, dst); err != nil { + return err + } + + return nil } // ConvertTo converts this CoreProviderList to the Hub version (v1alpha2). @@ -154,7 +218,19 @@ func (src *InfrastructureProvider) ConvertTo(dstRaw conversion.Hub) error { panic("expected to get an of object of type v1alpha2.InfrastructureProvider") } - return Convert_v1alpha1_InfrastructureProvider_To_v1alpha2_InfrastructureProvider(src, dst, nil) + if err := Convert_v1alpha1_InfrastructureProvider_To_v1alpha2_InfrastructureProvider(src, dst, nil); err != nil { + return err + } + + // Manually restore data. + restored := &operatorv1.InfrastructureProvider{} + if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok { + return err + } + + dst.Spec.ManifestPatches = restored.Spec.ManifestPatches + + return nil } // ConvertFrom converts from the InfrastructureProvider version (v1alpha2) to this version. @@ -164,7 +240,16 @@ func (dst *InfrastructureProvider) ConvertFrom(srcRaw conversion.Hub) error { panic("expected to get an of object of type v1alpha2.InfrastructureProvider") } - return Convert_v1alpha2_InfrastructureProvider_To_v1alpha1_InfrastructureProvider(src, dst, nil) + if err := Convert_v1alpha2_InfrastructureProvider_To_v1alpha1_InfrastructureProvider(src, dst, nil); err != nil { + return err + } + + // Preserve Hub data on down-conversion. + if err := utilconversion.MarshalData(src, dst); err != nil { + return err + } + + return nil } // ConvertTo converts this InfrastructureProviderList to the Hub version (v1alpha2). diff --git a/api/v1alpha1/zz_generated.conversion.go b/api/v1alpha1/zz_generated.conversion.go index 89e65dec..c0f86fc9 100644 --- a/api/v1alpha1/zz_generated.conversion.go +++ b/api/v1alpha1/zz_generated.conversion.go @@ -944,6 +944,7 @@ func autoConvert_v1alpha2_ProviderSpec_To_v1alpha1_ProviderSpec(in *v1alpha2.Pro // WARNING: in.ConfigSecret requires manual conversion: does not exist in peer-type out.FetchConfig = (*FetchConfiguration)(unsafe.Pointer(in.FetchConfig)) out.AdditionalManifestsRef = (*ConfigmapReference)(unsafe.Pointer(in.AdditionalManifestsRef)) + // WARNING: in.ManifestPatches requires manual conversion: does not exist in peer-type return nil } diff --git a/api/v1alpha2/provider_types.go b/api/v1alpha2/provider_types.go index 9620232e..d2bd0471 100644 --- a/api/v1alpha2/provider_types.go +++ b/api/v1alpha2/provider_types.go @@ -65,6 +65,14 @@ type ProviderSpec struct { // namespace of the provider will be used. There is no validation of the yaml content inside the configmap. // +optional AdditionalManifestsRef *ConfigmapReference `json:"additionalManifests,omitempty"` + + // ManifestPatches are applied to rendered provider manifests to customize the + // provider manifests. Patches are applied in the order they are specified. + // The `kind` field must match the target object, and + // if `apiVersion` is specified it will only be applied to matching objects. + // This should be an inline yaml blob-string https://datatracker.ietf.org/doc/html/rfc7396 + // +optional + ManifestPatches []string `json:"manifestPatches,omitempty"` } // ConfigmapReference contains enough information to locate the configmap. diff --git a/api/v1alpha2/zz_generated.deepcopy.go b/api/v1alpha2/zz_generated.deepcopy.go index 1b5da16b..4c81e750 100644 --- a/api/v1alpha2/zz_generated.deepcopy.go +++ b/api/v1alpha2/zz_generated.deepcopy.go @@ -786,6 +786,11 @@ func (in *ProviderSpec) DeepCopyInto(out *ProviderSpec) { *out = new(ConfigmapReference) **out = **in } + if in.ManifestPatches != nil { + in, out := &in.ManifestPatches, &out.ManifestPatches + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProviderSpec. diff --git a/config/crd/bases/operator.cluster.x-k8s.io_addonproviders.yaml b/config/crd/bases/operator.cluster.x-k8s.io_addonproviders.yaml index 2b1cb850..452e5a95 100644 --- a/config/crd/bases/operator.cluster.x-k8s.io_addonproviders.yaml +++ b/config/crd/bases/operator.cluster.x-k8s.io_addonproviders.yaml @@ -1466,6 +1466,15 @@ spec: type: integer type: object type: object + manifestPatches: + description: ManifestPatches are applied to rendered provider manifests + to customize the provider manifests. Patches are applied in the + order they are specified. The `kind` field must match the target + object, and if `apiVersion` is specified it will only be applied + to matching objects. This should be an inline yaml blob-string https://datatracker.ietf.org/doc/html/rfc7396 + items: + type: string + type: array version: description: Version indicates the provider version. type: string diff --git a/config/crd/bases/operator.cluster.x-k8s.io_bootstrapproviders.yaml b/config/crd/bases/operator.cluster.x-k8s.io_bootstrapproviders.yaml index 238f04bc..2e04be23 100644 --- a/config/crd/bases/operator.cluster.x-k8s.io_bootstrapproviders.yaml +++ b/config/crd/bases/operator.cluster.x-k8s.io_bootstrapproviders.yaml @@ -2995,6 +2995,15 @@ spec: type: integer type: object type: object + manifestPatches: + description: ManifestPatches are applied to rendered provider manifests + to customize the provider manifests. Patches are applied in the + order they are specified. The `kind` field must match the target + object, and if `apiVersion` is specified it will only be applied + to matching objects. This should be an inline yaml blob-string https://datatracker.ietf.org/doc/html/rfc7396 + items: + type: string + type: array version: description: Version indicates the provider version. type: string diff --git a/config/crd/bases/operator.cluster.x-k8s.io_controlplaneproviders.yaml b/config/crd/bases/operator.cluster.x-k8s.io_controlplaneproviders.yaml index b8495871..6e288462 100644 --- a/config/crd/bases/operator.cluster.x-k8s.io_controlplaneproviders.yaml +++ b/config/crd/bases/operator.cluster.x-k8s.io_controlplaneproviders.yaml @@ -2998,6 +2998,15 @@ spec: type: integer type: object type: object + manifestPatches: + description: ManifestPatches are applied to rendered provider manifests + to customize the provider manifests. Patches are applied in the + order they are specified. The `kind` field must match the target + object, and if `apiVersion` is specified it will only be applied + to matching objects. This should be an inline yaml blob-string https://datatracker.ietf.org/doc/html/rfc7396 + items: + type: string + type: array version: description: Version indicates the provider version. type: string diff --git a/config/crd/bases/operator.cluster.x-k8s.io_coreproviders.yaml b/config/crd/bases/operator.cluster.x-k8s.io_coreproviders.yaml index ee58fd53..b1d50c4d 100644 --- a/config/crd/bases/operator.cluster.x-k8s.io_coreproviders.yaml +++ b/config/crd/bases/operator.cluster.x-k8s.io_coreproviders.yaml @@ -2995,6 +2995,15 @@ spec: type: integer type: object type: object + manifestPatches: + description: ManifestPatches are applied to rendered provider manifests + to customize the provider manifests. Patches are applied in the + order they are specified. The `kind` field must match the target + object, and if `apiVersion` is specified it will only be applied + to matching objects. This should be an inline yaml blob-string https://datatracker.ietf.org/doc/html/rfc7396 + items: + type: string + type: array version: description: Version indicates the provider version. type: string diff --git a/config/crd/bases/operator.cluster.x-k8s.io_infrastructureproviders.yaml b/config/crd/bases/operator.cluster.x-k8s.io_infrastructureproviders.yaml index 5a57c38e..6d85fbbc 100644 --- a/config/crd/bases/operator.cluster.x-k8s.io_infrastructureproviders.yaml +++ b/config/crd/bases/operator.cluster.x-k8s.io_infrastructureproviders.yaml @@ -2998,6 +2998,15 @@ spec: type: integer type: object type: object + manifestPatches: + description: ManifestPatches are applied to rendered provider manifests + to customize the provider manifests. Patches are applied in the + order they are specified. The `kind` field must match the target + object, and if `apiVersion` is specified it will only be applied + to matching objects. This should be an inline yaml blob-string https://datatracker.ietf.org/doc/html/rfc7396 + items: + type: string + type: array version: description: Version indicates the provider version. type: string From cd6472f97d94ff25a9100c879a6acc835e3616dd Mon Sep 17 00:00:00 2001 From: alexander-demicev Date: Thu, 12 Oct 2023 20:30:18 +0200 Subject: [PATCH 2/5] Implement json patches for manifests Signed-off-by: alexander-demicev --- internal/controller/component_patches.go | 41 +++++ internal/controller/phases.go | 8 +- internal/patch/matchinfo.go | 44 ++++++ internal/patch/mergepatch.go | 51 ++++++ internal/patch/patch.go | 68 ++++++++ internal/patch/patch_test.go | 192 +++++++++++++++++++++++ internal/patch/resource.go | 101 ++++++++++++ 7 files changed, 503 insertions(+), 2 deletions(-) create mode 100644 internal/controller/component_patches.go create mode 100644 internal/patch/matchinfo.go create mode 100644 internal/patch/mergepatch.go create mode 100644 internal/patch/patch.go create mode 100644 internal/patch/patch_test.go create mode 100644 internal/patch/resource.go diff --git a/internal/controller/component_patches.go b/internal/controller/component_patches.go new file mode 100644 index 00000000..a2dd9982 --- /dev/null +++ b/internal/controller/component_patches.go @@ -0,0 +1,41 @@ +/* +Copyright 2023 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 controller + +import ( + "context" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/cluster-api-operator/internal/controller/genericprovider" + "sigs.k8s.io/cluster-api-operator/internal/patch" + ctrl "sigs.k8s.io/controller-runtime" +) + +func applyPatches(ctx context.Context, provider genericprovider.GenericProvider) func(objs []unstructured.Unstructured) ([]unstructured.Unstructured, error) { + log := ctrl.LoggerFrom(ctx) + + return func(objs []unstructured.Unstructured) ([]unstructured.Unstructured, error) { + if len(provider.GetSpec().ManifestPatches) == 0 { + log.V(5).Info("No resource patches to apply") + return objs, nil + } + + log.V(5).Info("Applying resource patches") + + return patch.ApplyPatches(objs, provider.GetSpec().ManifestPatches) + } +} diff --git a/internal/controller/phases.go b/internal/controller/phases.go index 1277c991..7729adfa 100644 --- a/internal/controller/phases.go +++ b/internal/controller/phases.go @@ -419,8 +419,12 @@ func (p *phaseReconciler) fetch(ctx context.Context) (reconcile.Result, error) { // ProviderSpec provides fields for customizing the provider deployment options. // We can use clusterctl library to apply this customizations. - err = repository.AlterComponents(p.components, customizeObjectsFn(p.provider)) - if err != nil { + if err := repository.AlterComponents(p.components, customizeObjectsFn(p.provider)); err != nil { + return reconcile.Result{}, wrapPhaseError(err, operatorv1.ComponentsFetchErrorReason) + } + + // Apply patches to the provider components if specified. + if err := repository.AlterComponents(p.components, applyPatches(ctx, p.provider)); err != nil { return reconcile.Result{}, wrapPhaseError(err, operatorv1.ComponentsFetchErrorReason) } diff --git a/internal/patch/matchinfo.go b/internal/patch/matchinfo.go new file mode 100644 index 00000000..bd143d57 --- /dev/null +++ b/internal/patch/matchinfo.go @@ -0,0 +1,44 @@ +/* +Copyright 2023 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 patch + +import ( + "fmt" + + "sigs.k8s.io/yaml" +) + +// we match resources and patches on their v1 TypeMeta. +type matchInfo struct { + Kind string `json:"kind,omitempty"` + APIVersion string `json:"apiVersion,omitempty"` + Metadata Metadata `json:"metadata,omitempty"` +} + +type Metadata struct { + Name string `json:"name,omitempty"` + Namespace string `json:"namespace,omitempty"` +} + +func parseYAMLMatchInfo(raw []byte) (matchInfo, error) { + m := matchInfo{} + if err := yaml.Unmarshal(raw, &m); err != nil { + return matchInfo{}, fmt.Errorf("failed to parse match info: %w", err) + } + + return m, nil +} diff --git a/internal/patch/mergepatch.go b/internal/patch/mergepatch.go new file mode 100644 index 00000000..c8da4897 --- /dev/null +++ b/internal/patch/mergepatch.go @@ -0,0 +1,51 @@ +/* +Copyright 2023 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 patch + +import ( + "fmt" + + "sigs.k8s.io/yaml" +) + +type mergePatch struct { + json []byte + matchInfo matchInfo +} + +func parseMergePatches(rawPatches []string) ([]mergePatch, error) { + patches := []mergePatch{} + + for _, patch := range rawPatches { + matchInfo, err := parseYAMLMatchInfo([]byte(patch)) + if err != nil { + return nil, fmt.Errorf("failed to parse patch: %w", err) + } + + json, err := yaml.YAMLToJSON([]byte(patch)) + if err != nil { + return nil, fmt.Errorf("failed to convert YAML to JSON: %w", err) + } + + patches = append(patches, mergePatch{ + json: json, + matchInfo: matchInfo, + }) + } + + return patches, nil +} diff --git a/internal/patch/patch.go b/internal/patch/patch.go new file mode 100644 index 00000000..1deba721 --- /dev/null +++ b/internal/patch/patch.go @@ -0,0 +1,68 @@ +/* +Copyright 2023 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 patch + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + utilyaml "sigs.k8s.io/cluster-api/util/yaml" + "sigs.k8s.io/yaml" +) + +// ApplyPatches patches a list of unstructured objects with a list of patches. +// Patches match if their kind and apiVersion match a document, with the exception +// that if the patch does not set apiVersion it will be ignored. +func ApplyPatches(toPatch []unstructured.Unstructured, patches []string) ([]unstructured.Unstructured, error) { + resources, err := parseResources(toPatch) + if err != nil { + return nil, fmt.Errorf("failed to parse resources: %w", err) + } + + mergePatches, err := parseMergePatches(patches) + if err != nil { + return nil, fmt.Errorf("failed to parse patches: %w", err) + } + + result := []unstructured.Unstructured{} + + for _, r := range resources { + for _, p := range mergePatches { + if _, err := r.applyMergePatch(p); err != nil { + return nil, fmt.Errorf("failed to apply patch: %w", err) + } + } + + r.patchedYAML, err = yaml.JSONToYAML(r.json) + if err != nil { + return nil, fmt.Errorf("failed to parse resource: %w", err) + } + + patchedObj, err := utilyaml.ToUnstructured(r.patchedYAML) + if err != nil { + return nil, fmt.Errorf("failed to parse resource: %w", err) + } + + if len(patchedObj) == 0 { + return nil, fmt.Errorf("patched object is empty") + } + + result = append(result, patchedObj...) + } + + return result, nil +} diff --git a/internal/patch/patch_test.go b/internal/patch/patch_test.go new file mode 100644 index 00000000..5987ab2b --- /dev/null +++ b/internal/patch/patch_test.go @@ -0,0 +1,192 @@ +/* +Copyright 2023 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 patch + +import ( + "testing" + + . "github.com/onsi/gomega" + utilyaml "sigs.k8s.io/cluster-api/util/yaml" +) + +func TestApplyPatches(t *testing.T) { + testCases := []struct { + name string + objectsToPatchYaml string + expectedPatchedObjectsYaml string + patches []string + expectedError bool + }{ + { + name: "should patch objects with multiple patches", + objectsToPatchYaml: testObjectsToPatchYaml, + expectedPatchedObjectsYaml: expectedTestPatchedObjectsYaml, + patches: []string{addServiceAccoungPatchRBAC, addLabelPatchService, removeSelectorPatchService, addSelectorPatchService, changePortOnSecondService}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + objectToPatch, err := utilyaml.ToUnstructured([]byte(tc.objectsToPatchYaml)) + g.Expect(err).NotTo(HaveOccurred()) + + result, err := ApplyPatches(objectToPatch, tc.patches) + if tc.expectedError { + g.Expect(err).To(HaveOccurred()) + } + g.Expect(err).NotTo(HaveOccurred()) + + resultYaml, err := utilyaml.FromUnstructured(result) + g.Expect(err).NotTo(HaveOccurred()) + + g.Expect(string(resultYaml)).To(Equal(tc.expectedPatchedObjectsYaml)) + }) + } +} + +const testObjectsToPatchYaml = `--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + labels: + some-label: value + name: rolebinding-name +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: role-name +subjects: +- kind: ServiceAccount + name: serviceaccount-name + namespace: namespace-name +--- +apiVersion: v1 +kind: Service +metadata: + labels: + some-label: value + name: service-name-1 + namespace: namespace-name +spec: + ports: + - port: 443 + targetPort: webhook-server + selector: + some-label: value +--- +apiVersion: v1 +kind: Service +metadata: + labels: + some-label: value + name: service-name-2 + namespace: namespace-name +spec: + ports: + - port: 443 + targetPort: webhook-server + selector: + some-label: value` + +const addServiceAccoungPatchRBAC = `apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +subjects: +- kind: ServiceAccount + name: serviceaccount-name + namespace: namespace-name +- kind: ServiceAccount + name: test-service-account + namespace: test-namespace` + +const addLabelPatchService = `--- +apiVersion: v1 +kind: Service +metadata: + labels: + test-label: test-value` + +const removeSelectorPatchService = `apiVersion: v1 +kind: Service +spec: + selector:` + +const addSelectorPatchService = `apiVersion: v1 +kind: Service +spec: + selector: + test-label: test-value` + +const changePortOnSecondService = `--- +apiVersion: v1 +kind: Service +metadata: + name: service-name-2 + namespace: namespace-name +spec: + ports: + - port: 7777 + targetPort: webhook-server` + +const expectedTestPatchedObjectsYaml = `apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + labels: + some-label: value + name: rolebinding-name +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: role-name +subjects: +- kind: ServiceAccount + name: serviceaccount-name + namespace: namespace-name +- kind: ServiceAccount + name: test-service-account + namespace: test-namespace +--- +apiVersion: v1 +kind: Service +metadata: + labels: + some-label: value + test-label: test-value + name: service-name-1 + namespace: namespace-name +spec: + ports: + - port: 443 + targetPort: webhook-server + selector: + test-label: test-value +--- +apiVersion: v1 +kind: Service +metadata: + labels: + some-label: value + test-label: test-value + name: service-name-2 + namespace: namespace-name +spec: + ports: + - port: 7777 + targetPort: webhook-server + selector: + test-label: test-value` diff --git a/internal/patch/resource.go b/internal/patch/resource.go new file mode 100644 index 00000000..62a1991f --- /dev/null +++ b/internal/patch/resource.go @@ -0,0 +1,101 @@ +/* +Copyright 2023 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 patch + +import ( + "fmt" + + jsonpatch "github.com/evanphx/json-patch/v5" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + utilyaml "sigs.k8s.io/cluster-api/util/yaml" + "sigs.k8s.io/yaml" +) + +type resource struct { + json []byte + patchedYAML []byte + matchInfo matchInfo +} + +func (r *resource) applyMergePatch(patch mergePatch) (matches bool, err error) { + if !r.matches(patch.matchInfo) { + return false, nil + } + + patched, err := jsonpatch.MergePatch(r.json, patch.json) + if err != nil { + return true, fmt.Errorf("failed to apply patch: %w", err) + } + + r.json = patched + + return true, nil +} + +func (r resource) matches(o matchInfo) bool { + m := &r.matchInfo + // we require kind to match, but if the patch does not specify + // APIVersion we ignore it. + if m.Kind != o.Kind { + return false + } + + // if api version not specified in patch we ignore it + if o.APIVersion == "" && m.APIVersion != o.APIVersion { + return false + } + + // if both namespace and name are specified in patch we require them to match + if o.Metadata.Namespace != "" && o.Metadata.Name != "" && m.Metadata.Namespace != o.Metadata.Namespace && m.Metadata.Name != o.Metadata.Name { + return false + } + + // if only name is specified in patch we require it to match(cluster scoped resources) + if o.Metadata.Name != "" && m.Metadata.Name != o.Metadata.Name { + return false + } + + return true +} + +func parseResources(toPatch []unstructured.Unstructured) ([]resource, error) { + resources := []resource{} + + for _, obj := range toPatch { + raw, err := utilyaml.FromUnstructured([]unstructured.Unstructured{obj}) + if err != nil { + return nil, fmt.Errorf("failed to parse resource: %w", err) + } + + matchInfo, err := parseYAMLMatchInfo(raw) + if err != nil { + return nil, fmt.Errorf("failed to parse resource: %w", err) + } + + json, err := yaml.YAMLToJSON(raw) + if err != nil { + return nil, fmt.Errorf("failted to parse resource: %w", err) + } + + resources = append(resources, resource{ + json: json, + matchInfo: matchInfo, + }) + } + + return resources, nil +} From 451800fde411f77cd35012ce56567932b3b203ce Mon Sep 17 00:00:00 2001 From: alexander-demicev Date: Thu, 12 Oct 2023 20:31:24 +0200 Subject: [PATCH 3/5] Add e2e tests for json patches Signed-off-by: alexander-demicev --- test/e2e/minimal_configuration_test.go | 23 +++++++++-- test/e2e/resources/full-chart-install.yaml | 45 ++++++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/test/e2e/minimal_configuration_test.go b/test/e2e/minimal_configuration_test.go index cb56430f..0195c45c 100644 --- a/test/e2e/minimal_configuration_test.go +++ b/test/e2e/minimal_configuration_test.go @@ -69,6 +69,11 @@ data: AdditionalManifestsRef: &operatorv1.ConfigmapReference{ Name: additionalManifests.Name, }, + ManifestPatches: []string{`apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + test-label: test-value`}, }, }, } @@ -83,6 +88,15 @@ data: Deployment: &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: coreProviderDeploymentName, Namespace: operatorNamespace}}, }, e2eConfig.GetIntervals(bootstrapClusterProxy.GetName(), "wait-controllers")...) + By("Checking for deployment to have additional labels") + deployment := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: coreProviderDeploymentName, Namespace: operatorNamespace}} + WaitFor(ctx, For(deployment).In(bootstrapCluster).ToSatisfy(func() bool { + if v, ok := deployment.Labels["test-label"]; ok { + return v == "test-value" + } + return false + }), e2eConfig.GetIntervals(bootstrapClusterProxy.GetName(), "wait-controllers")...) + By("Waiting for core provider to be ready") WaitFor(ctx, For(coreProvider).In(bootstrapCluster).ToSatisfy( HaveStatusCondition(&coreProvider.Status.Conditions, operatorv1.ProviderInstalledCondition)), @@ -94,10 +108,11 @@ data: }), e2eConfig.GetIntervals(bootstrapClusterProxy.GetName(), "wait-controllers")...) By("Checking if additional manifests are applied") - cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{ - Name: "test-config-map", - Namespace: operatorNamespace, - }} + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-config-map", + Namespace: operatorNamespace, + }} WaitFor(ctx, For(cm).In(bootstrapCluster).ToSatisfy(func() bool { value, ok := cm.Data["test"] return ok && value == "test" diff --git a/test/e2e/resources/full-chart-install.yaml b/test/e2e/resources/full-chart-install.yaml index c0ebbe81..23383af8 100644 --- a/test/e2e/resources/full-chart-install.yaml +++ b/test/e2e/resources/full-chart-install.yaml @@ -1482,6 +1482,15 @@ spec: type: integer type: object type: object + manifestPatches: + description: ManifestPatches are applied to rendered provider manifests + to customize the provider manifests. Patches are applied in the + order they are specified. The `kind` field must match the target + object, and if `apiVersion` is specified it will only be applied + to matching objects. This should be an inline yaml blob-string https://datatracker.ietf.org/doc/html/rfc7396 + items: + type: string + type: array version: description: Version indicates the provider version. type: string @@ -4566,6 +4575,15 @@ spec: type: integer type: object type: object + manifestPatches: + description: ManifestPatches are applied to rendered provider manifests + to customize the provider manifests. Patches are applied in the + order they are specified. The `kind` field must match the target + object, and if `apiVersion` is specified it will only be applied + to matching objects. This should be an inline yaml blob-string https://datatracker.ietf.org/doc/html/rfc7396 + items: + type: string + type: array version: description: Version indicates the provider version. type: string @@ -7653,6 +7671,15 @@ spec: type: integer type: object type: object + manifestPatches: + description: ManifestPatches are applied to rendered provider manifests + to customize the provider manifests. Patches are applied in the + order they are specified. The `kind` field must match the target + object, and if `apiVersion` is specified it will only be applied + to matching objects. This should be an inline yaml blob-string https://datatracker.ietf.org/doc/html/rfc7396 + items: + type: string + type: array version: description: Version indicates the provider version. type: string @@ -10738,6 +10765,15 @@ spec: type: integer type: object type: object + manifestPatches: + description: ManifestPatches are applied to rendered provider manifests + to customize the provider manifests. Patches are applied in the + order they are specified. The `kind` field must match the target + object, and if `apiVersion` is specified it will only be applied + to matching objects. This should be an inline yaml blob-string https://datatracker.ietf.org/doc/html/rfc7396 + items: + type: string + type: array version: description: Version indicates the provider version. type: string @@ -13825,6 +13861,15 @@ spec: type: integer type: object type: object + manifestPatches: + description: ManifestPatches are applied to rendered provider manifests + to customize the provider manifests. Patches are applied in the + order they are specified. The `kind` field must match the target + object, and if `apiVersion` is specified it will only be applied + to matching objects. This should be an inline yaml blob-string https://datatracker.ietf.org/doc/html/rfc7396 + items: + type: string + type: array version: description: Version indicates the provider version. type: string From 5c0a765d6bebe23d7b61379325cf2a235037d4be Mon Sep 17 00:00:00 2001 From: alexander-demicev Date: Thu, 12 Oct 2023 20:31:38 +0200 Subject: [PATCH 4/5] Document json patches usage Signed-off-by: alexander-demicev --- docs/README.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/docs/README.md b/docs/README.md index 1921b763..8c30d722 100644 --- a/docs/README.md +++ b/docs/README.md @@ -754,3 +754,35 @@ spec: additionalManifests: name: additional-manifests ``` + +## Patching provider manifests + +Provider manifests can be patched using JSON merge patches. This can be useful when you need to modify the provider manifests that are fetched from the repository. In order to provider +manifests `spec.ResourcePatches` has to be used where an array of patches can be specified: + +```yaml +--- +apiVersion: operator.cluster.x-k8s.io/v1alpha2 +kind: CoreProvider +metadata: + name: cluster-api + namespace: capi-system +spec: + resourcePatches: + - | +apiVersion: v1 +kind: Service +metadata: +labels: + test-label: test-value +``` + +More information about JSON merge patches can be found here https://datatracker.ietf.org/doc/html/rfc7396 + +There are couple of rules for the patch to match a manifest: + +- The `kind` field must match the target object. +- If `apiVersion` is specified it will only be applied to matching objects. +- If `metadata.name` and `metadata.namespace` not specified, the patch will be applied to all objects of the specified kind. +- If `metadata.name` is specified, the patch will be applied to the object with the specified name. This is for cluster scoped objects. +- If both `metadata.name` and `metadata.namespace` are specified, the patch will be applied to the object with the specified name and namespace. From 2fdc32d9135cfd3bf69a2c7cd9e525389f1d482c Mon Sep 17 00:00:00 2001 From: alexander-demicev Date: Mon, 30 Oct 2023 10:59:58 +0100 Subject: [PATCH 5/5] Update dependencies Signed-off-by: alexander-demicev --- go.mod | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 8fe751e7..ceead346 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ replace sigs.k8s.io/cluster-api => sigs.k8s.io/cluster-api v1.5.1 require ( github.com/MakeNowJust/heredoc v1.0.0 + github.com/evanphx/json-patch/v5 v5.6.0 github.com/google/go-cmp v0.6.0 github.com/google/go-github/v52 v52.0.0 github.com/google/gofuzz v1.2.0 @@ -23,6 +24,7 @@ require ( k8s.io/utils v0.0.0-20230209194617-a36077c30491 sigs.k8s.io/cluster-api v1.5.1 sigs.k8s.io/controller-runtime v0.15.2 + sigs.k8s.io/yaml v1.3.0 ) require ( @@ -45,7 +47,6 @@ require ( github.com/drone/envsubst/v2 v2.0.0-20210730161058-179042472c46 // indirect github.com/emicklei/go-restful/v3 v3.10.2 // indirect github.com/evanphx/json-patch v5.6.0+incompatible // indirect - github.com/evanphx/json-patch/v5 v5.6.0 // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/go-logr/logr v1.2.4 // indirect github.com/go-openapi/jsonpointer v0.19.6 // indirect @@ -109,5 +110,4 @@ require ( k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect - sigs.k8s.io/yaml v1.3.0 // indirect )