diff --git a/go.mod b/go.mod index c450932567d1c7709fe707d425745b7972d5e2c1..6dc5c27acacf73c6d27de209c8c54720fef01b30 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/google/uuid v1.1.2 github.com/onsi/ginkgo v1.16.5 github.com/onsi/gomega v1.17.0 - github.com/openshift/api v0.0.0-20211215120111-7c47a5f63470 + github.com/openshift/api v0.0.0-20211222145011-3bf13cf5081a github.com/openshift/client-go v0.0.0-20211209144617-7385dd6338e3 github.com/openshift/library-go v0.0.0-20211220195323-eca2c467c492 github.com/operator-framework/operator-sdk v0.5.1-0.20190301204940-c2efe6f74e7b diff --git a/go.sum b/go.sum index 78f1f4a6e005c90ed4ae93a9fa50bff2e4c1d70d..607279f19b8c545fa3c68870d6c4b0027fe89774 100644 --- a/go.sum +++ b/go.sum @@ -535,8 +535,8 @@ github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQ github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0= github.com/openshift/api v0.0.0-20211209135129-c58d9f695577/go.mod h1:DoslCwtqUpr3d/gsbq4ZlkaMEdYqKxuypsDjorcHhME= -github.com/openshift/api v0.0.0-20211215120111-7c47a5f63470 h1:kYVTSbYsfLxSBnK8Z2ZN+qgAdclXAf2mYVDyHDfxTZ0= -github.com/openshift/api v0.0.0-20211215120111-7c47a5f63470/go.mod h1:F/eU6jgr6Q2VhMu1mSpMmygxAELd7+BUxs3NHZ25jV4= +github.com/openshift/api v0.0.0-20211222145011-3bf13cf5081a h1:lJQ6mKlpoUn19dmaLY/ldRm2Mwi+WtTJ2MD6bteLa4o= +github.com/openshift/api v0.0.0-20211222145011-3bf13cf5081a/go.mod h1:F/eU6jgr6Q2VhMu1mSpMmygxAELd7+BUxs3NHZ25jV4= github.com/openshift/build-machinery-go v0.0.0-20210712174854-1bb7fd1518d3/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE= github.com/openshift/build-machinery-go v0.0.0-20211213093930-7e33a7eb4ce3/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE= github.com/openshift/client-go v0.0.0-20211209144617-7385dd6338e3 h1:SG1aqwleU6bGD0X4mhkTNupjVnByMYYuW4XbnCPavQU= diff --git a/install/0000_30_machine-api-operator_02_machine.crd.yaml b/install/0000_30_machine-api-operator_02_machine.crd.yaml index d1ff62cd674f28edfa307129d3542499e8fcc8b7..eebdf90c82e49531deeb204a68484f90ddf8d5b9 100644 --- a/install/0000_30_machine-api-operator_02_machine.crd.yaml +++ b/install/0000_30_machine-api-operator_02_machine.crd.yaml @@ -80,6 +80,9 @@ spec: items: description: LifecycleHook represents a single instance of a lifecycle hook type: object + required: + - name + - owner properties: name: description: Name defines a unique name for the lifcycle hook. The name should be unique and descriptive, ideally 1-3 words, in CamelCase or it may be namespaced, eg. foo.example.com/CamelCase. Names must be unique and should only be managed by a single entity. @@ -98,6 +101,9 @@ spec: items: description: LifecycleHook represents a single instance of a lifecycle hook type: object + required: + - name + - owner properties: name: description: Name defines a unique name for the lifcycle hook. The name should be unique and descriptive, ideally 1-3 words, in CamelCase or it may be namespaced, eg. foo.example.com/CamelCase. Names must be unique and should only be managed by a single entity. diff --git a/install/0000_30_machine-api-operator_03_machineset.crd.yaml b/install/0000_30_machine-api-operator_03_machineset.crd.yaml index 038802366a9237817e1695c3dcf33285bf028d0f..d61e4ed3c42351ece2435a3bf0dd23b03be0ece3 100644 --- a/install/0000_30_machine-api-operator_03_machineset.crd.yaml +++ b/install/0000_30_machine-api-operator_03_machineset.crd.yaml @@ -173,6 +173,9 @@ spec: items: description: LifecycleHook represents a single instance of a lifecycle hook type: object + required: + - name + - owner properties: name: description: Name defines a unique name for the lifcycle hook. The name should be unique and descriptive, ideally 1-3 words, in CamelCase or it may be namespaced, eg. foo.example.com/CamelCase. Names must be unique and should only be managed by a single entity. @@ -191,6 +194,9 @@ spec: items: description: LifecycleHook represents a single instance of a lifecycle hook type: object + required: + - name + - owner properties: name: description: Name defines a unique name for the lifcycle hook. The name should be unique and descriptive, ideally 1-3 words, in CamelCase or it may be namespaced, eg. foo.example.com/CamelCase. Names must be unique and should only be managed by a single entity. diff --git a/pkg/webhooks/machine_webhook.go b/pkg/webhooks/machine_webhook.go index d35e24d8eee78a7378b737739df31c6381f11874..8f1b7a2c00f1e2e4fd71db7ce9ed915bfc8e9fde 100644 --- a/pkg/webhooks/machine_webhook.go +++ b/pkg/webhooks/machine_webhook.go @@ -1276,18 +1276,40 @@ func isAzureGovCloud(platformStatus *osconfigv1.PlatformStatus) bool { func validateMachineLifecycleHooks(m, oldM *machinev1.Machine) []error { var errs []error - if !isDeleting(m) || oldM == nil { - return errs + + if nameErrs := checkUniqueHookNames(m.Spec.LifecycleHooks.PreDrain, field.NewPath("spec", "lifecycleHooks", "preDrain")); len(nameErrs) > 0 { + errs = append(errs, nameErrs...) + } + if nameErrs := checkUniqueHookNames(m.Spec.LifecycleHooks.PreTerminate, field.NewPath("spec", "lifecycleHooks", "preTerminate")); len(nameErrs) > 0 { + errs = append(errs, nameErrs...) } - changedPreDrain := lifecyclehooks.GetChangedLifecycleHooks(oldM.Spec.LifecycleHooks.PreDrain, m.Spec.LifecycleHooks.PreDrain) - if len(changedPreDrain) > 0 { - errs = append(errs, field.Forbidden(field.NewPath("spec", "lifecycleHooks", "preDrain"), fmt.Sprintf("pre-drain hooks are immutable when machine is marked for deletion: the following hooks are new or changed: %+v", changedPreDrain))) + if isDeleting(m) && oldM != nil { + changedPreDrain := lifecyclehooks.GetChangedLifecycleHooks(oldM.Spec.LifecycleHooks.PreDrain, m.Spec.LifecycleHooks.PreDrain) + if len(changedPreDrain) > 0 { + errs = append(errs, field.Forbidden(field.NewPath("spec", "lifecycleHooks", "preDrain"), fmt.Sprintf("pre-drain hooks are immutable when machine is marked for deletion: the following hooks are new or changed: %+v", changedPreDrain))) + } + + changedPreTerminate := lifecyclehooks.GetChangedLifecycleHooks(oldM.Spec.LifecycleHooks.PreTerminate, m.Spec.LifecycleHooks.PreTerminate) + if len(changedPreTerminate) > 0 { + errs = append(errs, field.Forbidden(field.NewPath("spec", "lifecycleHooks", "preTerminate"), fmt.Sprintf("pre-terminate hooks are immutable when machine is marked for deletion: the following hooks are new or changed: %+v", changedPreTerminate))) + } } - changedPreTerminate := lifecyclehooks.GetChangedLifecycleHooks(oldM.Spec.LifecycleHooks.PreTerminate, m.Spec.LifecycleHooks.PreTerminate) - if len(changedPreTerminate) > 0 { - errs = append(errs, field.Forbidden(field.NewPath("spec", "lifecycleHooks", "preTerminate"), fmt.Sprintf("pre-terminate hooks are immutable when machine is marked for deletion: the following hooks are new or changed: %+v", changedPreTerminate))) + return errs +} + +// checkUniqueHookNames checks that the names of hooks within a lifecycle stage are unique +func checkUniqueHookNames(hooks []machinev1.LifecycleHook, parent *field.Path) []error { + errs := []error{} + names := make(map[string]struct{}) + + for i, hook := range hooks { + if _, found := names[hook.Name]; found { + errs = append(errs, field.Forbidden(parent.Index(i).Child("name"), fmt.Sprintf("hook names must be unique within a lifecycle stage, the following hook name is already set: %s", hook.Name))) + } else { + names[hook.Name] = struct{}{} + } } return errs diff --git a/pkg/webhooks/machine_webhook_test.go b/pkg/webhooks/machine_webhook_test.go index e0e3be9d39b5dfe9980fe70ef4069b147fc62bb1..9ed5ba696e49463620af9f1bfbbf5105b913837f 100644 --- a/pkg/webhooks/machine_webhook_test.go +++ b/pkg/webhooks/machine_webhook_test.go @@ -817,6 +817,18 @@ func TestMachineUpdate(t *testing.T) { m.Spec.LifecycleHooks = machinev1.LifecycleHooks{} }, }, + { + name: "when duplicating a lifecycle hook", + platformType: osconfigv1.AWSPlatformType, + clusterID: awsClusterID, + baseProviderSpecValue: &kruntime.RawExtension{ + Object: defaultAWSProviderSpec.DeepCopy(), + }, + updateMachine: func(m *machinev1.Machine) { + m.Spec.LifecycleHooks.PreDrain = []machinev1.LifecycleHook{preDrainHook, preDrainHook} + }, + expectedError: "spec.lifecycleHooks.preDrain[1].name: Forbidden: hook names must be unique within a lifecycle stage, the following hook name is already set: pre-drain", + }, } for _, tc := range testCases { diff --git a/vendor/github.com/openshift/api/config/v1/0000_10_config-operator_01_infrastructure.crd.yaml b/vendor/github.com/openshift/api/config/v1/0000_10_config-operator_01_infrastructure.crd.yaml index 60b30a86562aa26fe24c1b42d7812b0555d7e1a9..6be6c681292b3487ee219e053be34151da4436ec 100644 --- a/vendor/github.com/openshift/api/config/v1/0000_10_config-operator_01_infrastructure.crd.yaml +++ b/vendor/github.com/openshift/api/config/v1/0000_10_config-operator_01_infrastructure.crd.yaml @@ -210,7 +210,7 @@ spec: resourceGroupID: description: resourceGroupID is the ID of the resource group for the cluster. type: string - pattern: ^rg-[0-9A-Za-z]+$ + pattern: ^(rg-[0-9A-Za-z]+)?$ resourceTags: description: resourceTags is a list of additional tags to apply to Alibaba Cloud resources created for the cluster. type: array diff --git a/vendor/github.com/openshift/api/config/v1/types_infrastructure.go b/vendor/github.com/openshift/api/config/v1/types_infrastructure.go index 016b1a3f9c321b7951e1919fb4b6a06e975b44fe..fe42bec8394d070e5b30d7f220465324cafca054 100644 --- a/vendor/github.com/openshift/api/config/v1/types_infrastructure.go +++ b/vendor/github.com/openshift/api/config/v1/types_infrastructure.go @@ -665,7 +665,7 @@ type AlibabaCloudPlatformStatus struct { // +required Region string `json:"region"` // resourceGroupID is the ID of the resource group for the cluster. - // +kubebuilder:validation:Pattern=`^rg-[0-9A-Za-z]+$` + // +kubebuilder:validation:Pattern=`^(rg-[0-9A-Za-z]+)?$` // +optional ResourceGroupID string `json:"resourceGroupID,omitempty"` // resourceTags is a list of additional tags to apply to Alibaba Cloud resources created for the cluster. diff --git a/vendor/github.com/openshift/api/machine/v1beta1/0000_10_machine.crd.yaml b/vendor/github.com/openshift/api/machine/v1beta1/0000_10_machine.crd.yaml index d1ff62cd674f28edfa307129d3542499e8fcc8b7..eebdf90c82e49531deeb204a68484f90ddf8d5b9 100644 --- a/vendor/github.com/openshift/api/machine/v1beta1/0000_10_machine.crd.yaml +++ b/vendor/github.com/openshift/api/machine/v1beta1/0000_10_machine.crd.yaml @@ -80,6 +80,9 @@ spec: items: description: LifecycleHook represents a single instance of a lifecycle hook type: object + required: + - name + - owner properties: name: description: Name defines a unique name for the lifcycle hook. The name should be unique and descriptive, ideally 1-3 words, in CamelCase or it may be namespaced, eg. foo.example.com/CamelCase. Names must be unique and should only be managed by a single entity. @@ -98,6 +101,9 @@ spec: items: description: LifecycleHook represents a single instance of a lifecycle hook type: object + required: + - name + - owner properties: name: description: Name defines a unique name for the lifcycle hook. The name should be unique and descriptive, ideally 1-3 words, in CamelCase or it may be namespaced, eg. foo.example.com/CamelCase. Names must be unique and should only be managed by a single entity. diff --git a/vendor/github.com/openshift/api/machine/v1beta1/0000_10_machineset.crd.yaml b/vendor/github.com/openshift/api/machine/v1beta1/0000_10_machineset.crd.yaml index 038802366a9237817e1695c3dcf33285bf028d0f..d61e4ed3c42351ece2435a3bf0dd23b03be0ece3 100644 --- a/vendor/github.com/openshift/api/machine/v1beta1/0000_10_machineset.crd.yaml +++ b/vendor/github.com/openshift/api/machine/v1beta1/0000_10_machineset.crd.yaml @@ -173,6 +173,9 @@ spec: items: description: LifecycleHook represents a single instance of a lifecycle hook type: object + required: + - name + - owner properties: name: description: Name defines a unique name for the lifcycle hook. The name should be unique and descriptive, ideally 1-3 words, in CamelCase or it may be namespaced, eg. foo.example.com/CamelCase. Names must be unique and should only be managed by a single entity. @@ -191,6 +194,9 @@ spec: items: description: LifecycleHook represents a single instance of a lifecycle hook type: object + required: + - name + - owner properties: name: description: Name defines a unique name for the lifcycle hook. The name should be unique and descriptive, ideally 1-3 words, in CamelCase or it may be namespaced, eg. foo.example.com/CamelCase. Names must be unique and should only be managed by a single entity. diff --git a/vendor/github.com/openshift/api/machine/v1beta1/types_machine.go b/vendor/github.com/openshift/api/machine/v1beta1/types_machine.go index 8d30407655267205002826f66f97810f1809e836..91e9d32dc09194389fa519f36a007d2e9cc5552d 100644 --- a/vendor/github.com/openshift/api/machine/v1beta1/types_machine.go +++ b/vendor/github.com/openshift/api/machine/v1beta1/types_machine.go @@ -222,7 +222,7 @@ type LifecycleHook struct { // +kubebuilder:validation:Pattern=`^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$` // +kubebuilder:validation:MinLength:=3 // +kubebuilder:validation:MaxLength:=256 - // +required + // +kubebuilder:validation:Required Name string `json:"name"` // Owner defines the owner of the lifecycle hook. @@ -232,7 +232,7 @@ type LifecycleHook struct { // or an administrator managing the hook. // +kubebuilder:validation:MinLength:=3 // +kubebuilder:validation:MaxLength:=512 - // +required + // +kubebuilder:validation:Required Owner string `json:"owner"` } diff --git a/vendor/modules.txt b/vendor/modules.txt index 73666c7755d367e53b58f002eb4a6a42d859702c..a6434d6da15fa2c574bc2753cd7b0a0053d2fbdd 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -187,7 +187,7 @@ github.com/onsi/gomega/matchers/support/goraph/edge github.com/onsi/gomega/matchers/support/goraph/node github.com/onsi/gomega/matchers/support/goraph/util github.com/onsi/gomega/types -# github.com/openshift/api v0.0.0-20211215120111-7c47a5f63470 +# github.com/openshift/api v0.0.0-20211222145011-3bf13cf5081a ## explicit; go 1.16 github.com/openshift/api github.com/openshift/api/apiserver