diff --git a/api/v1alpha1/clustertemplate_types.go b/api/v1alpha1/clustertemplate_types.go index 315d8c47e..25da38ed9 100644 --- a/api/v1alpha1/clustertemplate_types.go +++ b/api/v1alpha1/clustertemplate_types.go @@ -30,7 +30,8 @@ const ( // ClusterTemplateSpec defines the desired state of ClusterTemplate type ClusterTemplateSpec struct { - Helm HelmSpec `json:"helm"` + Helm HelmSpec `json:"helm"` + CAPIContracts CompatibilityContracts `json:"capiContracts,omitempty"` // Kubernetes exact version in the SemVer format provided by this ClusterTemplate. KubernetesVersion string `json:"k8sVersion,omitempty"` // Providers represent required CAPI providers with supported contract versions. @@ -41,6 +42,7 @@ type ClusterTemplateSpec struct { // ClusterTemplateStatus defines the observed state of ClusterTemplate type ClusterTemplateStatus struct { + CAPIContracts CompatibilityContracts `json:"capiContracts,omitempty"` // Kubernetes exact version in the SemVer format provided by this ClusterTemplate. KubernetesVersion string `json:"k8sVersion,omitempty"` // Providers represent required CAPI providers with supported contract versions @@ -53,7 +55,14 @@ type ClusterTemplateStatus struct { // FillStatusWithProviders sets the status of the template with providers // either from the spec or from the given annotations. func (t *ClusterTemplate) FillStatusWithProviders(annotations map[string]string) error { - t.Status.Providers = parseProviders(t, annotations) + t.Status.Providers = getProvidersList(t, annotations) + + contractsStatus, err := getCAPIContracts(t, annotations) + if err != nil { + return fmt.Errorf("failed to get CAPI contract versions for ClusterTemplate %s/%s: %v", t.GetNamespace(), t.GetName(), err) + } + + t.Status.CAPIContracts = contractsStatus kversion := annotations[ChartAnnotationKubernetesVersion] if t.Spec.KubernetesVersion != "" { @@ -64,7 +73,7 @@ func (t *ClusterTemplate) FillStatusWithProviders(annotations map[string]string) } if _, err := semver.NewVersion(kversion); err != nil { - return fmt.Errorf("failed to parse kubernetes version %s: %w", kversion, err) + return fmt.Errorf("failed to parse kubernetes version %s for ClusterTemplate %s/%s: %w", kversion, t.GetNamespace(), t.GetName(), err) } t.Status.KubernetesVersion = kversion @@ -72,6 +81,11 @@ func (t *ClusterTemplate) FillStatusWithProviders(annotations map[string]string) return nil } +// GetContracts returns .spec.capiContracts of the Template. +func (t *ClusterTemplate) GetContracts() CompatibilityContracts { + return t.Spec.CAPIContracts +} + // GetSpecProviders returns .spec.providers of the Template. func (t *ClusterTemplate) GetSpecProviders() Providers { return t.Spec.Providers diff --git a/api/v1alpha1/common.go b/api/v1alpha1/common.go index 3f6c8232a..31400432f 100644 --- a/api/v1alpha1/common.go +++ b/api/v1alpha1/common.go @@ -36,23 +36,16 @@ const ( ) type ( - // Holds different types of CAPI providers with [compatible contract versions]. - // - // [compatible contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts - Providers []NameContract + // Holds different types of CAPI providers. + Providers []string - // Represents name of the provider with its provider compatibility [contract versions]. + // Holds key-value pairs with compatibility [contract versions], + // where the key is the core CAPI contract version, + // and the value is an underscore-delimited (_) list of provider contract versions + // supported by the core CAPI. // // [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts - NameContract struct { - // Name of the provider. - Name string `json:"name,omitempty"` - // Compatibility restriction in the [CAPI provider format]. The value is an underscore-delimited (_) list of versions. - // Optional to be defined. - // - // [CAPI provider format]: https://cluster-api.sigs.k8s.io/developer/providers/contracts#api-version-labels - ContractVersion string `json:"contractVersion,omitempty"` - } + CompatibilityContracts map[string]string ) const ( @@ -128,12 +121,3 @@ func ExtractServiceTemplateName(rawObj client.Object) []string { return templates } - -// Names flattens the underlaying slice to provider names slice and returns it. -func (c Providers) Names() []string { - nn := make([]string, len(c)) - for i, v := range c { - nn[i] = v.Name - } - return nn -} diff --git a/api/v1alpha1/compatibility_contract.go b/api/v1alpha1/compatibility_contract.go new file mode 100644 index 000000000..632ebf913 --- /dev/null +++ b/api/v1alpha1/compatibility_contract.go @@ -0,0 +1,74 @@ +// Copyright 2024 +// +// 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 v1alpha1 + +import ( + "strconv" + "strings" +) + +// isCAPIContractVersion determines whether a given string +// represents a version in the CAPI contract version format (e.g. v1_v1beta1_v1alpha1, etc.). +func isCAPIContractVersion(version string) bool { + for _, v := range strings.Split(version, "_") { + if !isCAPIContractSingleVersion(v) { + return false + } + } + + return true +} + +// isCAPIContractSingleVersion determines whether a given string +// represents a single version in the CAPI contract version format (e.g. v1, v1beta1, v1alpha1, etc.). +func isCAPIContractSingleVersion(version string) bool { + if !strings.HasPrefix(version, "v") { + return false + } + + parts := strings.Split(version, "v") + if len(parts) != 2 || parts[0] != "" || strings.IndexByte(version, '_') != -1 { // skip v1_v1beta1 list of versions + return false + } + + const ( + alphaPrefix, betaPrefix = "alpha", "beta" + ) + + versionNumber := parts[1] + alphaIndex := strings.Index(versionNumber, alphaPrefix) + betaIndex := strings.Index(versionNumber, betaPrefix) + + if alphaIndex != -1 { + return isNonMajor(versionNumber, alphaPrefix, alphaIndex) + } else if betaIndex != -1 { + return isNonMajor(versionNumber, betaPrefix, betaIndex) + } + + _, err := strconv.Atoi(strings.TrimSpace(versionNumber)) + return err == nil +} + +func isNonMajor(version, prefix string, prefixIdx int) bool { + majorVer := version[:prefixIdx] + prefixedVer := version[prefixIdx+len(prefix):] + + if _, err := strconv.Atoi(majorVer); err != nil { + return false + } + + _, err := strconv.Atoi(prefixedVer) + return err == nil +} diff --git a/api/v1alpha1/compatibility_contract_test.go b/api/v1alpha1/compatibility_contract_test.go new file mode 100644 index 000000000..3bd889e6d --- /dev/null +++ b/api/v1alpha1/compatibility_contract_test.go @@ -0,0 +1,72 @@ +// Copyright 2024 +// +// 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 v1alpha1 + +import "testing" + +func Test_isCAPIContractVersion(t *testing.T) { + tests := []struct { + version string + isValid bool + }{ + {"v1", true}, + {"v1alpha1", true}, + {"v1beta1", true}, + {"v2", true}, + {"v3alpha2", true}, + {"v33beta22", true}, + {"v1alpha1_v1beta1", true}, + {"v1alpha1v1alha2_v1beta1", false}, + {"v4beta1", true}, + {"invalid", false}, + {"v1alpha", false}, + {"v1beta", false}, + {"v1alpha1beta1", false}, + } + + for _, test := range tests { + result := isCAPIContractVersion(test.version) + if result != test.isValid { + t.Errorf("isValidVersion(%q) = %v, want %v", test.version, result, test.isValid) + } + } +} + +func Test_isCAPIContractSingleVersion(t *testing.T) { + tests := []struct { + version string + isValid bool + }{ + {"v1", true}, + {"v1alpha1", true}, + {"v1beta1", true}, + {"v2", true}, + {"v3alpha2", true}, + {"v33beta22", true}, + {"v4beta1", true}, + {"invalid", false}, + {"v1alpha", false}, + {"v1beta", false}, + {"v1alpha1beta1", false}, + {"v1alpha1_v1beta1", false}, + } + + for _, test := range tests { + result := isCAPIContractSingleVersion(test.version) + if result != test.isValid { + t.Errorf("isValidVersion(%q) = %v, want %v", test.version, result, test.isValid) + } + } +} diff --git a/api/v1alpha1/management_types.go b/api/v1alpha1/management_types.go index bc1adf74b..fdbd9b1a7 100644 --- a/api/v1alpha1/management_types.go +++ b/api/v1alpha1/management_types.go @@ -87,6 +87,8 @@ func GetDefaultProviders() []Provider { // ManagementStatus defines the observed state of Management type ManagementStatus struct { + // TODO: + CAPIContracts map[string]CompatibilityContracts `json:"capiContracts,omitempty"` // Components indicates the status of installed HMC components and CAPI providers. Components map[string]ComponentStatus `json:"components,omitempty"` // Release indicates the current Release object. diff --git a/api/v1alpha1/providertemplate_types.go b/api/v1alpha1/providertemplate_types.go index 15f682a8e..ad102bfcf 100644 --- a/api/v1alpha1/providertemplate_types.go +++ b/api/v1alpha1/providertemplate_types.go @@ -15,23 +15,15 @@ package v1alpha1 import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) + "fmt" -const ( - // ChartAnnotationCAPIContractVersion is an annotation containing the expected core CAPI contract version (e.g. v1beta1) associated with a ProviderTemplate. - ChartAnnotationCAPIContractVersion = "hmc.mirantis.com/capi-version" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // ProviderTemplateSpec defines the desired state of ProviderTemplate type ProviderTemplateSpec struct { - Helm HelmSpec `json:"helm,omitempty"` - // CAPI [contract version] indicating compatibility with the core CAPI. - // Currently supported versions: v1alpha3_v1alpha4_v1beta1. - // The field is not applicable for the cluster-api ProviderTemplate. - // - // [contract version]: https://cluster-api.sigs.k8s.io/developer/providers/contracts - CAPIContractVersion string `json:"capiContractVersion,omitempty"` + Helm HelmSpec `json:"helm,omitempty"` + CAPIContracts CompatibilityContracts `json:"capiContracts,omitempty"` // Providers represent exposed CAPI providers with supported contract versions. // Should be set if not present in the Helm chart metadata. // Supported contract versions are optional to be defined. @@ -40,11 +32,7 @@ type ProviderTemplateSpec struct { // ProviderTemplateStatus defines the observed state of ProviderTemplate type ProviderTemplateStatus struct { - // CAPI [contract version] indicating compatibility with the core CAPI. - // Currently supported versions: v1alpha3_v1alpha4_v1beta1. - // - // [contract version]: https://cluster-api.sigs.k8s.io/developer/providers/contracts - CAPIContractVersion string `json:"capiContractVersion,omitempty"` + CAPIContracts CompatibilityContracts `json:"capiContracts,omitempty"` // Providers represent exposed CAPI providers with supported contract versions // if the latter has been given. Providers Providers `json:"providers,omitempty"` @@ -55,22 +43,23 @@ type ProviderTemplateStatus struct { // FillStatusWithProviders sets the status of the template with providers // either from the spec or from the given annotations. func (t *ProviderTemplate) FillStatusWithProviders(annotations map[string]string) error { - t.Status.Providers = parseProviders(t, annotations) + t.Status.Providers = getProvidersList(t, annotations) - if t.Name == CoreCAPIName { - return nil + contractsStatus, err := getCAPIContracts(t, annotations) + if err != nil { + return fmt.Errorf("failed to get CAPI contract versions for ProviderTemplate %s: %v", t.GetName(), err) } - requiredCAPIContract := annotations[ChartAnnotationCAPIContractVersion] - if t.Spec.CAPIContractVersion != "" { - requiredCAPIContract = t.Spec.CAPIContractVersion - } - - t.Status.CAPIContractVersion = requiredCAPIContract + t.Status.CAPIContracts = contractsStatus return nil } +// GetContracts returns .spec.capiContracts of the Template. +func (t *ProviderTemplate) GetContracts() CompatibilityContracts { + return t.Spec.CAPIContracts +} + // GetSpecProviders returns .spec.providers of the Template. func (t *ProviderTemplate) GetSpecProviders() Providers { return t.Spec.Providers diff --git a/api/v1alpha1/servicetemplate_types.go b/api/v1alpha1/servicetemplate_types.go index 7bf0d5142..ddca28629 100644 --- a/api/v1alpha1/servicetemplate_types.go +++ b/api/v1alpha1/servicetemplate_types.go @@ -16,7 +16,6 @@ package v1alpha1 import ( "fmt" - "strings" "github.com/Masterminds/semver/v3" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -52,19 +51,7 @@ type ServiceTemplateStatus struct { // FillStatusWithProviders sets the status of the template with providers // either from the spec or from the given annotations. func (t *ServiceTemplate) FillStatusWithProviders(annotations map[string]string) error { - providers := annotations[ChartAnnotationProviderName] - if len(providers) == 0 { - t.Status.Providers = t.Spec.Providers - } else { - splitted := strings.Split(providers, multiProviderSeparator) - t.Status.Providers = make(Providers, 0, len(splitted)) - t.Status.Providers = append(t.Status.Providers, t.Spec.Providers...) - for _, v := range splitted { - if c := strings.TrimSpace(v); c != "" { - t.Status.Providers = append(t.Status.Providers, NameContract{Name: c}) - } - } - } + t.Status.Providers = getProvidersList(t, annotations) kconstraint := annotations[ChartAnnotationKubernetesConstraint] if t.Spec.KubernetesConstraint != "" { @@ -75,7 +62,7 @@ func (t *ServiceTemplate) FillStatusWithProviders(annotations map[string]string) } if _, err := semver.NewConstraint(kconstraint); err != nil { - return fmt.Errorf("failed to parse kubernetes constraint %s: %w", kconstraint, err) + return fmt.Errorf("failed to parse kubernetes constraint %s for ServiceTemplate %s/%s: %w", kconstraint, t.GetNamespace(), t.GetName(), err) } t.Status.KubernetesConstraint = kconstraint @@ -83,6 +70,11 @@ func (t *ServiceTemplate) FillStatusWithProviders(annotations map[string]string) return nil } +// GetSpecProviders returns .spec.providers of the Template. +func (t *ServiceTemplate) GetSpecProviders() Providers { + return t.Spec.Providers +} + // GetHelmSpec returns .spec.helm of the Template. func (t *ServiceTemplate) GetHelmSpec() *HelmSpec { return &t.Spec.Helm diff --git a/api/v1alpha1/templates_common.go b/api/v1alpha1/templates_common.go index 5fe0ebfe8..0ec9d8b8e 100644 --- a/api/v1alpha1/templates_common.go +++ b/api/v1alpha1/templates_common.go @@ -15,6 +15,9 @@ package v1alpha1 import ( + "errors" + "fmt" + "slices" "strings" helmcontrollerv2 "github.com/fluxcd/helm-controller/api/v2" @@ -25,6 +28,8 @@ const ( // ChartAnnotationProviderName is the annotation set on components in a Template. // This annotations allows to identify all the components belonging to a provider. ChartAnnotationProviderName = "cluster.x-k8s.io/provider" + + chartAnnoCAPIPrefix = "cluster.x-k8s.io/" ) // +kubebuilder:validation:XValidation:rule="(has(self.chartName) && !has(self.chartRef)) || (!has(self.chartName) && has(self.chartRef))", message="either chartName or chartRef must be set" @@ -72,36 +77,76 @@ type TemplateValidationStatus struct { Valid bool `json:"valid"` } -const multiProviderSeparator = "," +func getProvidersList(providersGetter interface{ GetSpecProviders() Providers }, annotations map[string]string) Providers { + const multiProviderSeparator = "," + + if spec := providersGetter.GetSpecProviders(); len(spec) > 0 { + slices.Sort(spec) + return slices.Compact(spec) + } -func parseProviders(providersGetter interface{ GetSpecProviders() Providers }, annotations map[string]string) []NameContract { providers := annotations[ChartAnnotationProviderName] if len(providers) == 0 { - return providersGetter.GetSpecProviders() + return Providers{} } var ( - ps = providersGetter.GetSpecProviders() - splitted = strings.Split(providers, multiProviderSeparator) - pstatus = make([]NameContract, 0, len(splitted)+len(ps)) + pstatus = make([]string, 0, len(splitted)) ) - pstatus = append(pstatus, ps...) - for _, v := range splitted { - v = strings.TrimSpace(v) - nameContract := strings.SplitN(v, " ", 2) - if len(nameContract) == 0 { // BCE (bound check elimination) - continue + if c := strings.TrimSpace(v); c != "" { + pstatus = append(pstatus, c) + } + } + + return pstatus +} + +func getCAPIContracts(contractsGetter interface{ GetContracts() CompatibilityContracts }, annotations map[string]string) (_ CompatibilityContracts, merr error) { + contractsStatus := make(map[string]string) + + // spec preceding the annos + if contracts := contractsGetter.GetContracts(); len(contracts) > 0 { + for capiContract, providerContract := range contracts { + if !isCAPIContractSingleVersion(capiContract) { + merr = errors.Join(merr, fmt.Errorf("incorrect CAPI contract version %s in the spec", capiContract)) + continue + } + + if providerContract != "" && !isCAPIContractVersion(providerContract) { // special case for either CAPI or deliberately set empty + merr = errors.Join(merr, fmt.Errorf("incorrect provider contract version %s in the spec for the %s CAPI contract version", providerContract, capiContract)) + continue + } + + contractsStatus[capiContract] = providerContract } - n := NameContract{Name: nameContract[0]} - if len(nameContract) > 1 { - n.ContractVersion = nameContract[1] + return contractsStatus, merr + } + + for k, providerContract := range annotations { + idx := strings.Index(k, chartAnnoCAPIPrefix) + if idx < 0 { + continue } - pstatus = append(pstatus, n) + capiContract := k[idx+len(chartAnnoCAPIPrefix):] + if isCAPIContractSingleVersion(capiContract) { + if providerContract == "" { // special case for either CAPI or deliberately set empty + contractsStatus[capiContract] = "" + continue + } + + if isCAPIContractVersion(providerContract) { + contractsStatus[capiContract] = providerContract + } else { + // since we parsed capi contract version, + // then treat the provider's invalid version as an error + merr = errors.Join(merr, fmt.Errorf("incorrect provider contract version %s given for the %s CAPI contract version annotation", providerContract, k)) + } + } } - return pstatus + return contractsStatus, merr } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 543a99e4d..01b001861 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -188,6 +188,13 @@ func (in *ClusterTemplateList) DeepCopyObject() runtime.Object { func (in *ClusterTemplateSpec) DeepCopyInto(out *ClusterTemplateSpec) { *out = *in in.Helm.DeepCopyInto(&out.Helm) + if in.CAPIContracts != nil { + in, out := &in.CAPIContracts, &out.CAPIContracts + *out = make(CompatibilityContracts, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.Providers != nil { in, out := &in.Providers, &out.Providers *out = make(Providers, len(*in)) @@ -208,6 +215,13 @@ func (in *ClusterTemplateSpec) DeepCopy() *ClusterTemplateSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterTemplateStatus) DeepCopyInto(out *ClusterTemplateStatus) { *out = *in + if in.CAPIContracts != nil { + in, out := &in.CAPIContracts, &out.CAPIContracts + *out = make(CompatibilityContracts, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.Providers != nil { in, out := &in.Providers, &out.Providers *out = make(Providers, len(*in)) @@ -226,6 +240,27 @@ func (in *ClusterTemplateStatus) DeepCopy() *ClusterTemplateStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in CompatibilityContracts) DeepCopyInto(out *CompatibilityContracts) { + { + in := &in + *out = make(CompatibilityContracts, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CompatibilityContracts. +func (in CompatibilityContracts) DeepCopy() CompatibilityContracts { + if in == nil { + return nil + } + out := new(CompatibilityContracts) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Component) DeepCopyInto(out *Component) { *out = *in @@ -604,6 +639,24 @@ func (in *ManagementSpec) DeepCopy() *ManagementSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ManagementStatus) DeepCopyInto(out *ManagementStatus) { *out = *in + if in.CAPIContracts != nil { + in, out := &in.CAPIContracts, &out.CAPIContracts + *out = make(map[string]CompatibilityContracts, len(*in)) + for key, val := range *in { + var outVal map[string]string + if val == nil { + (*out)[key] = nil + } else { + inVal := (*in)[key] + in, out := &inVal, &outVal + *out = make(CompatibilityContracts, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + (*out)[key] = outVal + } + } if in.Components != nil { in, out := &in.Components, &out.Components *out = make(map[string]ComponentStatus, len(*in)) @@ -725,21 +778,6 @@ func (in *MultiClusterServiceStatus) DeepCopy() *MultiClusterServiceStatus { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *NameContract) DeepCopyInto(out *NameContract) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NameContract. -func (in *NameContract) DeepCopy() *NameContract { - if in == nil { - return nil - } - out := new(NameContract) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NamedProviderTemplate) DeepCopyInto(out *NamedProviderTemplate) { *out = *in @@ -835,6 +873,13 @@ func (in *ProviderTemplateList) DeepCopyObject() runtime.Object { func (in *ProviderTemplateSpec) DeepCopyInto(out *ProviderTemplateSpec) { *out = *in in.Helm.DeepCopyInto(&out.Helm) + if in.CAPIContracts != nil { + in, out := &in.CAPIContracts, &out.CAPIContracts + *out = make(CompatibilityContracts, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.Providers != nil { in, out := &in.Providers, &out.Providers *out = make(Providers, len(*in)) @@ -855,6 +900,13 @@ func (in *ProviderTemplateSpec) DeepCopy() *ProviderTemplateSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ProviderTemplateStatus) DeepCopyInto(out *ProviderTemplateStatus) { *out = *in + if in.CAPIContracts != nil { + in, out := &in.CAPIContracts, &out.CAPIContracts + *out = make(CompatibilityContracts, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.Providers != nil { in, out := &in.Providers, &out.Providers *out = make(Providers, len(*in)) diff --git a/internal/controller/managedcluster_controller.go b/internal/controller/managedcluster_controller.go index 121218040..dfbad613a 100644 --- a/internal/controller/managedcluster_controller.go +++ b/internal/controller/managedcluster_controller.go @@ -587,8 +587,8 @@ func (r *ManagedClusterReconciler) getInfraProvidersNames(ctx context.Context, t ips := make([]string, 0, len(template.Status.Providers)) for _, v := range template.Status.Providers { - if strings.HasPrefix(v.Name, "infrastructure-") { - ips = append(ips, v.Name) + if strings.HasPrefix(v, "infrastructure-") { + ips = append(ips, v) } } diff --git a/internal/controller/managedcluster_controller_test.go b/internal/controller/managedcluster_controller_test.go index 9a8f9bfd0..4d93fcce9 100644 --- a/internal/controller/managedcluster_controller_test.go +++ b/internal/controller/managedcluster_controller_test.go @@ -94,7 +94,7 @@ var _ = Describe("ManagedCluster Controller", func() { Raw: []byte(`{"foo":"bar"}`), }, }, - Providers: hmc.Providers{{Name: "infrastructure-aws"}}, + Providers: hmc.Providers{"infrastructure-aws"}, } Expect(k8sClient.Status().Update(ctx, template)).To(Succeed()) } @@ -110,7 +110,7 @@ var _ = Describe("ManagedCluster Controller", func() { } Expect(k8sClient.Create(ctx, management)).To(Succeed()) management.Status = hmc.ManagementStatus{ - AvailableProviders: hmc.Providers{{Name: "infrastructure-aws"}}, + AvailableProviders: hmc.Providers{"infrastructure-aws"}, } Expect(k8sClient.Status().Update(ctx, management)).To(Succeed()) } diff --git a/internal/controller/management_controller.go b/internal/controller/management_controller.go index dc75f9d83..b2e906e5f 100644 --- a/internal/controller/management_controller.go +++ b/internal/controller/management_controller.go @@ -20,7 +20,6 @@ import ( "errors" "fmt" "slices" - "strings" fluxv2 "github.com/fluxcd/helm-controller/api/v2" "github.com/fluxcd/pkg/apis/meta" @@ -106,9 +105,13 @@ func (r *ManagementReconciler) Update(ctx context.Context, management *hmc.Manag return ctrl.Result{}, err } - var errs error - detectedProviders := hmc.Providers{} - detectedComponents := make(map[string]hmc.ComponentStatus) + var ( + errs error + + detectedProviders = hmc.Providers{} + detectedComponents = make(map[string]hmc.ComponentStatus) + detectedContracts = make(map[string]hmc.CompatibilityContracts) + ) err := r.enableAdditionalComponents(ctx, management) if err != nil { @@ -128,13 +131,13 @@ func (r *ManagementReconciler) Update(ctx context.Context, management *hmc.Manag }, template) if err != nil { errMsg := fmt.Sprintf("Failed to get ProviderTemplate %s: %s", component.Template, err) - updateComponentsStatus(detectedComponents, &detectedProviders, component.helmReleaseName, component.Template, template.Status.Providers, errMsg) + updateComponentsStatus(detectedComponents, &detectedProviders, detectedContracts, component.helmReleaseName, component.Template, template.Status.Providers, template.Status.CAPIContracts, errMsg) errs = errors.Join(errs, errors.New(errMsg)) continue } if !template.Status.Valid { errMsg := fmt.Sprintf("Template %s is not marked as valid", component.Template) - updateComponentsStatus(detectedComponents, &detectedProviders, component.helmReleaseName, component.Template, template.Status.Providers, errMsg) + updateComponentsStatus(detectedComponents, &detectedProviders, detectedContracts, component.helmReleaseName, component.Template, template.Status.Providers, template.Status.CAPIContracts, errMsg) errs = errors.Join(errs, errors.New(errMsg)) continue } @@ -148,15 +151,16 @@ func (r *ManagementReconciler) Update(ctx context.Context, management *hmc.Manag }) if err != nil { errMsg := fmt.Sprintf("error reconciling HelmRelease %s/%s: %s", r.SystemNamespace, component.Template, err) - updateComponentsStatus(detectedComponents, &detectedProviders, component.helmReleaseName, component.Template, template.Status.Providers, errMsg) + updateComponentsStatus(detectedComponents, &detectedProviders, detectedContracts, component.helmReleaseName, component.Template, template.Status.Providers, template.Status.CAPIContracts, errMsg) errs = errors.Join(errs, errors.New(errMsg)) continue } - updateComponentsStatus(detectedComponents, &detectedProviders, component.helmReleaseName, component.Template, template.Status.Providers, "") + updateComponentsStatus(detectedComponents, &detectedProviders, detectedContracts, component.helmReleaseName, component.Template, template.Status.Providers, template.Status.CAPIContracts, "") } management.Status.ObservedGeneration = management.Generation management.Status.AvailableProviders = detectedProviders + management.Status.CAPIContracts = detectedContracts management.Status.Components = detectedComponents management.Status.Release = management.Spec.Release if err := r.Status().Update(ctx, management); err != nil { @@ -415,9 +419,11 @@ func (r *ManagementReconciler) enableAdditionalComponents(ctx context.Context, m func updateComponentsStatus( components map[string]hmc.ComponentStatus, providers *hmc.Providers, + capiContracts map[string]hmc.CompatibilityContracts, componentName string, templateName string, templateProviders hmc.Providers, + templateContracts hmc.CompatibilityContracts, err string, ) { components[componentName] = hmc.ComponentStatus{ @@ -426,14 +432,14 @@ func updateComponentsStatus( Template: templateName, } - // TODO: do we actually need to partially include supported versions? The validation is taking - // place only when the actual provider template had been created, so either we have to perform - // validation here in the controller or change templates status via the validation webhook. - // Or just use the fixed versions, so no partially supported contracts would exist. if err == "" { *providers = append(*providers, templateProviders...) - slices.SortFunc(*providers, func(a, b hmc.NameContract) int { return strings.Compare(a.Name, b.Name) }) + slices.Sort(*providers) *providers = slices.Compact(*providers) + + for _, v := range templateProviders { + capiContracts[v] = templateContracts // TODO (zerospiel): not sure whether it's okay to overwrite if the same provider + } } } diff --git a/internal/controller/template_controller.go b/internal/controller/template_controller.go index 73f218354..8310c6ada 100644 --- a/internal/controller/template_controller.go +++ b/internal/controller/template_controller.go @@ -341,64 +341,58 @@ func (r *ClusterTemplateReconciler) validateCompatibilityAttrs(ctx context.Conte exposedProviders, requiredProviders := management.Status.AvailableProviders, template.Status.Providers - ctrl.LoggerFrom(ctx).V(1).Info("providers to check", "exposed", exposedProviders, "required", requiredProviders) - - var merr error - missing, wrong := collectMissingProvidersWithWrongVersions(exposedProviders, requiredProviders) - merr = errors.Join(merr, missing, wrong) - - if merr != nil { - _ = r.updateStatus(ctx, template, merr.Error()) - return merr - } - - return r.updateStatus(ctx, template, "") -} - -// collectMissingProvidersWithWrongVersions returns collected errors for missing providers and providers with -// unsatisfying contract versions. -func collectMissingProvidersWithWrongVersions(exposed, required []hmc.NameContract) (missingErr, nonSatisfyingErr error) { - exposed2Contracts := make(map[string][]string, len(exposed)) - for _, v := range exposed { - if v.ContractVersion != "" { - exposed2Contracts[v.Name] = strings.Split(v.ContractVersion, "_") - } else { - exposed2Contracts[v.Name] = nil - } - } - - var missing, nonSatisfying []string - for _, requiredProvider := range required { - exposedContracts, ok := exposed2Contracts[requiredProvider.Name] - if !ok { - missing = append(missing, requiredProvider.Name) + ctrl.LoggerFrom(ctx).V(1).Info("providers to check", "exposed", exposedProviders, "required", requiredProviders, + "exposed_capi_contract_versions", management.Status.CAPIContracts, "required_capi_contract_versions", template.Status.CAPIContracts) + + var ( + merr error + missing []string + nonSatisfying []string + ) + for _, v := range requiredProviders { + if !slices.Contains(exposedProviders, v) { + missing = append(missing, v) continue } - if len(exposedContracts) == 0 || requiredProvider.ContractVersion == "" { - continue + providerCAPIContracts, ok := management.Status.CAPIContracts[v] + if !ok { + continue // both the provider and cluster templates contract versions must be set for the validation } - requiredContracts := strings.Split(requiredProvider.ContractVersion, "_") // must be only one, but better to check twice + // already validated contract versions format + for capi, providerReq := range template.Status.CAPIContracts { + providerSupported, ok := providerCAPIContracts[capi] + if !ok { + // TODO (zerospiel): should we also consider it as a missing error? capi req from cluster missing in provider tpl + continue + } - for _, rc := range requiredContracts { - if !slices.Contains(exposedContracts, rc) { - nonSatisfying = append(nonSatisfying, fmt.Sprintf("%s %s !~ %v", requiredProvider.Name, rc, exposedContracts)) + providerSupportedContracts := strings.Split(providerSupported, "_") + for _, v := range strings.Split(providerReq, "_") { + if !slices.Contains(providerSupportedContracts, v) { + nonSatisfying = append(nonSatisfying, v) + } } } } if len(missing) > 0 { slices.Sort(missing) - missingErr = fmt.Errorf("one or more required providers are not deployed yet: %v", missing) + merr = errors.Join(merr, fmt.Errorf("one or more required providers are not deployed yet: %v", missing)) } if len(nonSatisfying) > 0 { slices.Sort(nonSatisfying) - nonSatisfyingErr = fmt.Errorf("one or more required providers does not satisfy constraints: %v", nonSatisfying) + merr = errors.Join(merr, fmt.Errorf("one or more required provider contract versions does not satisfy deployed: %v", nonSatisfying)) } - return missingErr, nonSatisfyingErr + if merr != nil { + _ = r.updateStatus(ctx, template, merr.Error()) + return merr + } + + return r.updateStatus(ctx, template, "") } // SetupWithManager sets up the controller with the Manager. diff --git a/internal/controller/template_controller_test.go b/internal/controller/template_controller_test.go index 5d011426f..dc1c38991 100644 --- a/internal/controller/template_controller_test.go +++ b/internal/controller/template_controller_test.go @@ -201,8 +201,9 @@ var _ = Describe("Template Controller", func() { clusterTemplateName = "cluster-template-test-name" mgmtName = hmcmirantiscomv1alpha1.ManagementName someProviderName = "test-provider-name" - someRequiredContract = "v1beta1" + someRequiredContract = "v1beta2" someExposedContract = "v1beta1_v1beta2" + capiVersion = "v1beta1" timeout = time.Second * 10 interval = time.Millisecond * 250 @@ -216,13 +217,9 @@ var _ = Describe("Template Controller", func() { Namespace: metav1.NamespaceDefault, }, Spec: hmcmirantiscomv1alpha1.ClusterTemplateSpec{ - Helm: helmSpec, - Providers: []hmcmirantiscomv1alpha1.NameContract{ - { - Name: someProviderName, - ContractVersion: someRequiredContract, - }, - }, + Helm: helmSpec, + Providers: []string{someProviderName}, + CAPIContracts: hmcmirantiscomv1alpha1.CompatibilityContracts{capiVersion: someRequiredContract}, }, } Expect(k8sClient.Create(ctx, clusterTemplate)).To(Succeed()) @@ -236,9 +233,15 @@ var _ = Describe("Template Controller", func() { if l := len(clusterTemplate.Spec.Providers); l != 1 { return fmt.Errorf("expected .spec.providers length to be exactly 1, got %d", l) } + if l := len(clusterTemplate.Spec.CAPIContracts); l != 1 { + return fmt.Errorf("expected .spec.capiContracts length to be exactly 1, got %d", l) + } - if v := clusterTemplate.Spec.Providers[0]; v.Name != someProviderName || v.ContractVersion != someRequiredContract { - return fmt.Errorf("expected .spec.providers[0] to be %s:%s, got %s:%s", someProviderName, someRequiredContract, v.Name, v.ContractVersion) + if v := clusterTemplate.Spec.Providers[0]; v != someProviderName { + return fmt.Errorf("expected .spec.providers[0] to be %s, got %s", someProviderName, v) + } + if v := clusterTemplate.Spec.CAPIContracts[capiVersion]; v != someRequiredContract { + return fmt.Errorf("expected .spec.capiContracts[%s] to be %s, got %s", capiVersion, someRequiredContract, v) } return nil @@ -253,12 +256,8 @@ var _ = Describe("Template Controller", func() { } Expect(k8sClient.Create(ctx, mgmt)).To(Succeed()) mgmt.Status = hmcmirantiscomv1alpha1.ManagementStatus{ - AvailableProviders: []hmcmirantiscomv1alpha1.NameContract{ - { - Name: someProviderName, - ContractVersion: someExposedContract, - }, - }, + AvailableProviders: []string{someProviderName}, + CAPIContracts: map[string]hmcmirantiscomv1alpha1.CompatibilityContracts{someProviderName: {capiVersion: someExposedContract}}, } Expect(k8sClient.Status().Update(ctx, mgmt)).To(Succeed()) @@ -271,9 +270,15 @@ var _ = Describe("Template Controller", func() { if l := len(mgmt.Status.AvailableProviders); l != 1 { return fmt.Errorf("expected .status.availableProviders length to be exactly 1, got %d", l) } + if l := len(mgmt.Status.CAPIContracts); l != 1 { + return fmt.Errorf("expected .status.capiContracts length to be exactly 1, got %d", l) + } - if v := mgmt.Status.AvailableProviders[0]; v.Name != someProviderName || v.ContractVersion != someExposedContract { - return fmt.Errorf("expected .status.availableProviders[0] to be %s:%s, got %s:%s", someProviderName, someExposedContract, v.Name, v.ContractVersion) + if v := mgmt.Status.AvailableProviders[0]; v != someProviderName { + return fmt.Errorf("expected .status.availableProviders[0] to be %s, got %s", someProviderName, v) + } + if v := mgmt.Status.CAPIContracts[someProviderName]; v[capiVersion] != someExposedContract { + return fmt.Errorf("expected .status.capiContracts[%s][%s] to be %s, got %s", someProviderName, capiVersion, someExposedContract, v[capiVersion]) } return nil @@ -294,7 +299,9 @@ var _ = Describe("Template Controller", func() { Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(clusterTemplate), clusterTemplate)).To(Succeed()) Expect(clusterTemplate.Status.Valid && clusterTemplate.Status.ValidationError == "").To(BeTrue()) Expect(clusterTemplate.Status.Providers).To(HaveLen(1)) - Expect(clusterTemplate.Status.Providers[0]).To(Equal(hmcmirantiscomv1alpha1.NameContract{Name: someProviderName, ContractVersion: someRequiredContract})) + Expect(clusterTemplate.Status.CAPIContracts).To(HaveLen(1)) + Expect(clusterTemplate.Status.Providers[0]).To(Equal(someProviderName)) + Expect(clusterTemplate.Status.CAPIContracts).To(BeEquivalentTo(map[string]string{capiVersion: someRequiredContract})) By("Removing the created objects") Expect(k8sClient.Delete(ctx, mgmt)).To(Succeed()) diff --git a/internal/telemetry/tracker.go b/internal/telemetry/tracker.go index 8621dd23a..79dcf1934 100644 --- a/internal/telemetry/tracker.go +++ b/internal/telemetry/tracker.go @@ -92,7 +92,7 @@ func (t *Tracker) trackManagedClusterHeartbeat(ctx context.Context) error { clusterID, managedCluster.Spec.Template, template.Spec.Helm.ChartVersion, - template.Status.Providers.Names(), + template.Status.Providers, ) if err != nil { errs = errors.Join(errs, fmt.Errorf("failed to track the heartbeat of the managedcluster %s/%s", managedCluster.Namespace, managedCluster.Name)) diff --git a/internal/webhook/managedcluster_webhook.go b/internal/webhook/managedcluster_webhook.go index 6884f7416..672bc10ea 100644 --- a/internal/webhook/managedcluster_webhook.go +++ b/internal/webhook/managedcluster_webhook.go @@ -219,7 +219,7 @@ func (v *ManagedClusterValidator) validateCredential(ctx context.Context, manage hasInfra := false for _, v := range template.Status.Providers { - if strings.HasPrefix(v.Name, "infrastructure-") { + if strings.HasPrefix(v, "infrastructure-") { hasInfra = true break } @@ -249,24 +249,24 @@ func isCredMatchTemplate(cred *hmcv1alpha1.Credential, template *hmcv1alpha1.Clu } for _, provider := range template.Status.Providers { - switch provider.Name { + switch provider { case "infrastructure-aws": if idtyKind != "AWSClusterStaticIdentity" && idtyKind != "AWSClusterRoleIdentity" && idtyKind != "AWSClusterControllerIdentity" { - return errMsg(provider.Name) + return errMsg(provider) } case "infrastructure-azure": if idtyKind != "AzureClusterIdentity" { - return errMsg(provider.Name) + return errMsg(provider) } case "infrastructure-vsphere": if idtyKind != "VSphereClusterIdentity" { - return errMsg(provider.Name) + return errMsg(provider) } default: - if strings.HasPrefix(provider.Name, "infrastructure-") { - return fmt.Errorf("unsupported infrastructure provider %s", provider.Name) + if strings.HasPrefix(provider, "infrastructure-") { + return fmt.Errorf("unsupported infrastructure provider %s", provider) } } } diff --git a/internal/webhook/managedcluster_webhook_test.go b/internal/webhook/managedcluster_webhook_test.go index 398f3a507..fd1d76132 100644 --- a/internal/webhook/managedcluster_webhook_test.go +++ b/internal/webhook/managedcluster_webhook_test.go @@ -43,9 +43,9 @@ var ( mgmt = management.NewManagement( management.WithAvailableProviders(v1alpha1.Providers{ - {Name: "infrastructure-aws"}, - {Name: "control-plane-k0s"}, - {Name: "bootstrap-k0s"}, + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", }), ) @@ -128,9 +128,9 @@ func TestManagedClusterValidateCreate(t *testing.T) { template.NewClusterTemplate( template.WithName(testTemplateName), template.WithProvidersStatus(v1alpha1.Providers{ - {Name: "infrastructure-aws"}, - {Name: "control-plane-k0s"}, - {Name: "bootstrap-k0s"}, + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", }), template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), ), @@ -145,9 +145,9 @@ func TestManagedClusterValidateCreate(t *testing.T) { existingObjects: []runtime.Object{ cred, management.NewManagement(management.WithAvailableProviders(v1alpha1.Providers{ - {Name: "infrastructure-aws"}, - {Name: "control-plane-k0s"}, - {Name: "bootstrap-k0s"}, + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", })), template.NewClusterTemplate( template.WithName(testTemplateName), @@ -171,9 +171,9 @@ func TestManagedClusterValidateCreate(t *testing.T) { template.NewClusterTemplate( template.WithName(testTemplateName), template.WithProvidersStatus(v1alpha1.Providers{ - {Name: "infrastructure-aws"}, - {Name: "control-plane-k0s"}, - {Name: "bootstrap-k0s"}, + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", }), template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), ), @@ -200,9 +200,9 @@ func TestManagedClusterValidateCreate(t *testing.T) { template.NewClusterTemplate( template.WithName(testTemplateName), template.WithProvidersStatus(v1alpha1.Providers{ - {Name: "infrastructure-aws"}, - {Name: "control-plane-k0s"}, - {Name: "bootstrap-k0s"}, + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", }), template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), ), @@ -219,17 +219,17 @@ func TestManagedClusterValidateCreate(t *testing.T) { cred, management.NewManagement( management.WithAvailableProviders(v1alpha1.Providers{ - {Name: "infrastructure-azure"}, - {Name: "control-plane-k0s"}, - {Name: "bootstrap-k0s"}, + "infrastructure-azure", + "control-plane-k0smotron", + "bootstrap-k0smotron", }), ), template.NewClusterTemplate( template.WithName(testTemplateName), template.WithProvidersStatus(v1alpha1.Providers{ - {Name: "infrastructure-azure"}, - {Name: "control-plane-k0s"}, - {Name: "bootstrap-k0s"}, + "infrastructure-azure", + "control-plane-k0smotron", + "bootstrap-k0smotron", }), template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), ), @@ -311,9 +311,9 @@ func TestManagedClusterValidateUpdate(t *testing.T) { ValidationError: "validation error example", }), template.WithProvidersStatus(v1alpha1.Providers{ - {Name: "infrastructure-aws"}, - {Name: "control-plane-k0s"}, - {Name: "bootstrap-k0s"}, + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", }), ), }, diff --git a/internal/webhook/management_webhook.go b/internal/webhook/management_webhook.go index c15f38cfd..78f349c82 100644 --- a/internal/webhook/management_webhook.go +++ b/internal/webhook/management_webhook.go @@ -18,8 +18,6 @@ import ( "context" "errors" "fmt" - "slices" - "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -79,18 +77,18 @@ func (v *ManagementValidator) ValidateUpdate(ctx context.Context, _, newObj runt capiTplName = mgmt.Spec.Core.CAPI.Template } - supportedCAPIContractVersions := []string{"v1alpha3", "v1alpha4", "v1beta1"} - // TODO: i think it's better to just have a simple list instead of parcing a CRD on each update event + less groups for the rbac - // clusterCAPI := new(crdv1.CustomResourceDefinition) - // if err := v.Get(ctx, client.ObjectKey{Name: "clusters.cluster.x-k8s.io"}, clusterCAPI); err != nil { - // return nil, fmt.Errorf("failed to get Cluster CRD: %w", err) - // } + capiTpl := new(hmcv1alpha1.ProviderTemplate) + if err := v.Get(ctx, client.ObjectKey{Name: capiTplName}, capiTpl); err != nil { + return nil, fmt.Errorf("failed to get ProviderTemplate %s: %w", capiTplName, err) + } + + if len(capiTpl.Status.CAPIContracts) == 0 { + return nil, nil // nothing to validate against + } - // supportedCAPIContractVersions := make([]string, len(clusterCAPI.Spec.Versions)) - // for _, v := range clusterCAPI.Spec.Versions { - // // TODO: we can actually skip the depreceted just in case - // supportedCAPIContractVersions = append(supportedCAPIContractVersions, v.Name) - // } + if !capiTpl.Status.Valid { + return nil, fmt.Errorf("%s: not valid ProviderTemplate %s", invalidMgmtMsg, capiTpl.Name) + } var wrongVersions error for _, p := range mgmt.Spec.Providers { @@ -99,7 +97,7 @@ func (v *ManagementValidator) ValidateUpdate(ctx context.Context, _, newObj runt tplName = release.ProviderTemplate(p.Name) } - if tplName == capiTplName { // skip capi itself + if tplName == capiTpl.Name { // skip capi itself continue } @@ -108,14 +106,17 @@ func (v *ManagementValidator) ValidateUpdate(ctx context.Context, _, newObj runt return nil, fmt.Errorf("failed to get ProviderTemplate %s: %w", tplName, err) } - if pTpl.Status.CAPIContractVersion == "" { + if len(pTpl.Status.CAPIContracts) == 0 { continue } - expectedCAPIContractVersions := strings.Split(pTpl.Status.CAPIContractVersion, "_") - for _, ev := range expectedCAPIContractVersions { - if !slices.Contains(supportedCAPIContractVersions, ev) { - wrongVersions = errors.Join(wrongVersions, fmt.Errorf("core CAPI contract versions %v does not support ProviderTemplate %s contract %s", supportedCAPIContractVersions, pTpl.Name, ev)) + if !pTpl.Status.Valid { + return nil, fmt.Errorf("%s: not valid ProviderTemplate %s", invalidMgmtMsg, tplName) + } + + for capi := range capiTpl.Status.CAPIContracts { + if _, ok := pTpl.Status.CAPIContracts[capi]; !ok { + wrongVersions = errors.Join(wrongVersions, fmt.Errorf("core CAPI contract version %s does not support ProviderTemplate %s", capi, pTpl.Name)) } } } diff --git a/internal/webhook/management_webhook_test.go b/internal/webhook/management_webhook_test.go index e56eccf5a..4987dcf99 100644 --- a/internal/webhook/management_webhook_test.go +++ b/internal/webhook/management_webhook_test.go @@ -39,10 +39,13 @@ func TestManagementValidateUpdate(t *testing.T) { ctx := admission.NewContextWithRequest(context.Background(), admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Update}}) const ( - contractVersionValid = "v1alpha4_v1beta1" - contractVersionInvalid = "v1alpha1_v1alpha4_v1beta1" + someContractVersion = "v1alpha4_v1beta1" + capiVersion = "v1beta1" + capiVersionOther = "v1alpha3" ) + validStatus := v1alpha1.TemplateValidationStatus{Valid: true} + providerAwsDefaultTpl := v1alpha1.Provider{ Name: "infrastructure-aws", Component: v1alpha1.Component{ @@ -50,8 +53,6 @@ func TestManagementValidateUpdate(t *testing.T) { }, } - supportedCAPIContractVersions := []string{"v1alpha3", "v1alpha4", "v1beta1"} - tests := []struct { name string management *v1alpha1.Management @@ -64,13 +65,46 @@ func TestManagementValidateUpdate(t *testing.T) { management: management.NewManagement(), }, { - name: "no providertemplates having providers in mgmt spec, should fail", + name: "no capi providertemplate, should fail", + management: management.NewManagement(management.WithRelease(release.DefaultName)), + existingObjects: []runtime.Object{release.New()}, + err: fmt.Sprintf(`failed to get ProviderTemplate %s: providertemplates.hmc.mirantis.com "%s" not found`, release.DefaultCAPITemplateName, release.DefaultCAPITemplateName), + }, + { + name: "capi providertemplate without capi version set, should succeed", + management: management.NewManagement(management.WithRelease(release.DefaultName)), + existingObjects: []runtime.Object{ + release.New(), + template.NewProviderTemplate(template.WithName(release.DefaultCAPITemplateName)), + }, + }, + { + name: "capi providertemplate is not valid, should fail", + management: management.NewManagement(management.WithRelease(release.DefaultName)), + existingObjects: []runtime.Object{ + release.New(), + template.NewProviderTemplate( + template.WithName(release.DefaultCAPITemplateName), + template.WithProviderStatusCAPIContracts(capiVersion, ""), + ), + }, + err: fmt.Sprintf("the Management is invalid: not valid ProviderTemplate %s", release.DefaultCAPITemplateName), + }, + { + name: "no providertemplates that declared in mgmt spec.providers, should fail", management: management.NewManagement( management.WithRelease(release.DefaultName), management.WithProviders([]v1alpha1.Provider{providerAwsDefaultTpl}), ), - existingObjects: []runtime.Object{release.New()}, - err: fmt.Sprintf(`failed to get ProviderTemplate %s: providertemplates.hmc.mirantis.com "%s" not found`, template.DefaultName, template.DefaultName), + existingObjects: []runtime.Object{ + release.New(), + template.NewProviderTemplate( + template.WithName(release.DefaultCAPITemplateName), + template.WithProviderStatusCAPIContracts(capiVersion, ""), + template.WithValidationStatus(validStatus), + ), + }, + err: fmt.Sprintf(`failed to get ProviderTemplate %s: providertemplates.hmc.mirantis.com "%s" not found`, template.DefaultName, template.DefaultName), }, { name: "providertemplates without specified capi contracts, should succeed", @@ -82,10 +116,31 @@ func TestManagementValidateUpdate(t *testing.T) { release.New(), template.NewProviderTemplate( template.WithName(release.DefaultCAPITemplateName), + template.WithProviderStatusCAPIContracts(capiVersion, ""), + template.WithValidationStatus(validStatus), ), template.NewProviderTemplate(), }, }, + { + name: "providertemplates is not ready, should succeed", + management: management.NewManagement( + management.WithRelease(release.DefaultName), + management.WithProviders([]v1alpha1.Provider{providerAwsDefaultTpl}), + ), + existingObjects: []runtime.Object{ + release.New(), + template.NewProviderTemplate( + template.WithName(release.DefaultCAPITemplateName), + template.WithProviderStatusCAPIContracts(capiVersion, ""), + template.WithValidationStatus(validStatus), + ), + template.NewProviderTemplate( + template.WithProviderStatusCAPIContracts(capiVersionOther, someContractVersion), + ), + }, + err: fmt.Sprintf("the Management is invalid: not valid ProviderTemplate %s", template.DefaultName), + }, { name: "providertemplates do not match capi contracts, should fail", management: management.NewManagement( @@ -96,13 +151,16 @@ func TestManagementValidateUpdate(t *testing.T) { release.New(), template.NewProviderTemplate( template.WithName(release.DefaultCAPITemplateName), + template.WithProviderStatusCAPIContracts(capiVersion, ""), + template.WithValidationStatus(validStatus), ), template.NewProviderTemplate( - template.WithProviderStatusCAPIContract(contractVersionInvalid), + template.WithProviderStatusCAPIContracts(capiVersionOther, someContractVersion), + template.WithValidationStatus(validStatus), ), }, warnings: admission.Warnings{"The Management object has incompatible CAPI contract versions in ProviderTemplates"}, - err: fmt.Sprintf("the Management is invalid: core CAPI contract versions %v does not support ProviderTemplate %s contract %s", supportedCAPIContractVersions, template.DefaultName, "v1alpha1"), + err: fmt.Sprintf("the Management is invalid: core CAPI contract version %s does not support ProviderTemplate %s", capiVersion, template.DefaultName), }, { name: "providertemplates match capi contracts, should succeed", @@ -114,9 +172,12 @@ func TestManagementValidateUpdate(t *testing.T) { release.New(), template.NewProviderTemplate( template.WithName(release.DefaultCAPITemplateName), + template.WithProviderStatusCAPIContracts(capiVersion, ""), + template.WithValidationStatus(validStatus), ), template.NewProviderTemplate( - template.WithProviderStatusCAPIContract(contractVersionValid), + template.WithProviderStatusCAPIContracts(capiVersion, someContractVersion), + template.WithValidationStatus(validStatus), ), }, }, diff --git a/templates/cluster/aws-hosted-cp/Chart.yaml b/templates/cluster/aws-hosted-cp/Chart.yaml index 10c1fc142..0d6656d0f 100644 --- a/templates/cluster/aws-hosted-cp/Chart.yaml +++ b/templates/cluster/aws-hosted-cp/Chart.yaml @@ -14,4 +14,4 @@ version: 0.0.2 # It is recommended to use it with quotes. appVersion: "v1.31.1+k0s.1" annotations: - cluster.x-k8s.io/provider: infrastructure-aws; control-plane-k0smotron; bootstrap-k0s + cluster.x-k8s.io/provider: infrastructure-aws, control-plane-k0smotron, bootstrap-k0smotron diff --git a/templates/cluster/aws-standalone-cp/Chart.yaml b/templates/cluster/aws-standalone-cp/Chart.yaml index 93edbe18e..beebc934b 100644 --- a/templates/cluster/aws-standalone-cp/Chart.yaml +++ b/templates/cluster/aws-standalone-cp/Chart.yaml @@ -13,4 +13,4 @@ version: 0.0.2 # It is recommended to use it with quotes. appVersion: "v1.31.1+k0s.1" annotations: - cluster.x-k8s.io/provider: infrastructure-aws; control-plane-k0s; bootstrap-k0s + cluster.x-k8s.io/provider: infrastructure-aws, control-plane-k0smotron, bootstrap-k0smotron diff --git a/templates/cluster/azure-hosted-cp/Chart.yaml b/templates/cluster/azure-hosted-cp/Chart.yaml index 46dcca48f..b6c8ff28c 100644 --- a/templates/cluster/azure-hosted-cp/Chart.yaml +++ b/templates/cluster/azure-hosted-cp/Chart.yaml @@ -14,4 +14,4 @@ version: 0.0.3 # It is recommended to use it with quotes. appVersion: "v1.31.1+k0s.1" annotations: - cluster.x-k8s.io/provider: infrastructure-azure; control-plane-k0s; bootstrap-k0s + cluster.x-k8s.io/provider: infrastructure-azure, control-plane-k0smotron, bootstrap-k0smotron diff --git a/templates/cluster/azure-standalone-cp/Chart.yaml b/templates/cluster/azure-standalone-cp/Chart.yaml index e094bfea0..94294173c 100644 --- a/templates/cluster/azure-standalone-cp/Chart.yaml +++ b/templates/cluster/azure-standalone-cp/Chart.yaml @@ -13,4 +13,4 @@ version: 0.0.3 # It is recommended to use it with quotes. appVersion: "1.31.1+k0s.1" annotations: - cluster.x-k8s.io/provider: infrastructure-azure; control-plane-k0s; bootstrap-k0s + cluster.x-k8s.io/provider: infrastructure-azure, control-plane-k0smotron, bootstrap-k0smotron diff --git a/templates/cluster/vsphere-hosted-cp/Chart.yaml b/templates/cluster/vsphere-hosted-cp/Chart.yaml index e33bb4d3c..c6d97d674 100644 --- a/templates/cluster/vsphere-hosted-cp/Chart.yaml +++ b/templates/cluster/vsphere-hosted-cp/Chart.yaml @@ -14,5 +14,5 @@ version: 0.0.2 # It is recommended to use it with quotes. appVersion: "v1.31.1+k0s.1" annotations: - cluster.x-k8s.io/provider: infrastructure-vsphere; control-plane-k0s; bootstrap-k0s + cluster.x-k8s.io/provider: infrastructure-vsphere, control-plane-k0smotron, bootstrap-k0smotron hmc.mirantis.com/type: deployment diff --git a/templates/cluster/vsphere-standalone-cp/Chart.yaml b/templates/cluster/vsphere-standalone-cp/Chart.yaml index 49b29322f..9a6e77398 100644 --- a/templates/cluster/vsphere-standalone-cp/Chart.yaml +++ b/templates/cluster/vsphere-standalone-cp/Chart.yaml @@ -13,5 +13,5 @@ version: 0.0.2 # It is recommended to use it with quotes. appVersion: "v1.31.1+k0s.1" annotations: - cluster.x-k8s.io/provider: infrastructure-vsphere; control-plane-k0s; bootstrap-k0s + cluster.x-k8s.io/provider: infrastructure-vsphere, control-plane-k0smotron, bootstrap-k0smotron hmc.mirantis.com/type: deployment diff --git a/templates/provider/hmc/templates/crds/hmc.mirantis.com_clustertemplates.yaml b/templates/provider/hmc/templates/crds/hmc.mirantis.com_clustertemplates.yaml index fafcd2e0e..9d38efd6f 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_clustertemplates.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_clustertemplates.yaml @@ -56,6 +56,17 @@ spec: spec: description: ClusterTemplateSpec defines the desired state of ClusterTemplate properties: + capiContracts: + additionalProperties: + type: string + description: |- + Holds key-value pairs with compatibility [contract versions], + where the key is the core CAPI contract version, + and the value is an underscore-delimited (_) list of provider contract versions + supported by the core CAPI. + + [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts + type: object helm: description: HelmSpec references a Helm chart representing the HMC template @@ -113,22 +124,7 @@ spec: Should be set if not present in the Helm chart metadata. Compatibility attributes are optional to be defined. items: - description: |- - Represents name of the provider with its provider compatibility [contract versions]. - - [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts - properties: - contractVersion: - description: |- - Compatibility restriction in the [CAPI provider format]. The value is an underscore-delimited (_) list of versions. - Optional to be defined. - - [CAPI provider format]: https://cluster-api.sigs.k8s.io/developer/providers/contracts#api-version-labels - type: string - name: - description: Name of the provider. - type: string - type: object + type: string type: array required: - helm @@ -139,6 +135,17 @@ spec: status: description: ClusterTemplateStatus defines the observed state of ClusterTemplate properties: + capiContracts: + additionalProperties: + type: string + description: |- + Holds key-value pairs with compatibility [contract versions], + where the key is the core CAPI contract version, + and the value is an underscore-delimited (_) list of provider contract versions + supported by the core CAPI. + + [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts + type: object chartRef: description: |- ChartRef is a reference to a source controller resource containing the @@ -190,22 +197,7 @@ spec: Providers represent required CAPI providers with supported contract versions if the latter has been given. items: - description: |- - Represents name of the provider with its provider compatibility [contract versions]. - - [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts - properties: - contractVersion: - description: |- - Compatibility restriction in the [CAPI provider format]. The value is an underscore-delimited (_) list of versions. - Optional to be defined. - - [CAPI provider format]: https://cluster-api.sigs.k8s.io/developer/providers/contracts#api-version-labels - type: string - name: - description: Name of the provider. - type: string - type: object + type: string type: array valid: description: Valid indicates whether the template passed validation diff --git a/templates/provider/hmc/templates/crds/hmc.mirantis.com_managements.yaml b/templates/provider/hmc/templates/crds/hmc.mirantis.com_managements.yaml index f8df43e48..ac9280975 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_managements.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_managements.yaml @@ -116,23 +116,21 @@ spec: AvailableProviders holds all CAPI providers available along with their supported contract versions, if specified in ProviderTemplates, on the Management cluster. items: + type: string + type: array + capiContracts: + additionalProperties: + additionalProperties: + type: string description: |- - Represents name of the provider with its provider compatibility [contract versions]. + Holds key-value pairs with compatibility [contract versions], + where the key is the core CAPI contract version, + and the value is an underscore-delimited (_) list of provider contract versions + supported by the core CAPI. [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts - properties: - contractVersion: - description: |- - Compatibility restriction in the [CAPI provider format]. The value is an underscore-delimited (_) list of versions. - Optional to be defined. - - [CAPI provider format]: https://cluster-api.sigs.k8s.io/developer/providers/contracts#api-version-labels - type: string - name: - description: Name of the provider. - type: string type: object - type: array + type: object components: additionalProperties: description: ComponentStatus is the status of Management component diff --git a/templates/provider/hmc/templates/crds/hmc.mirantis.com_providertemplates.yaml b/templates/provider/hmc/templates/crds/hmc.mirantis.com_providertemplates.yaml index a50fbeb2d..b6c8751ad 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_providertemplates.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_providertemplates.yaml @@ -56,14 +56,17 @@ spec: spec: description: ProviderTemplateSpec defines the desired state of ProviderTemplate properties: - capiContractVersion: + capiContracts: + additionalProperties: + type: string description: |- - CAPI [contract version] indicating compatibility with the core CAPI. - Currently supported versions: v1alpha3_v1alpha4_v1beta1. - The field is not applicable for the cluster-api ProviderTemplate. + Holds key-value pairs with compatibility [contract versions], + where the key is the core CAPI contract version, + and the value is an underscore-delimited (_) list of provider contract versions + supported by the core CAPI. - [contract version]: https://cluster-api.sigs.k8s.io/developer/providers/contracts - type: string + [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts + type: object helm: description: HelmSpec references a Helm chart representing the HMC template @@ -117,22 +120,7 @@ spec: Should be set if not present in the Helm chart metadata. Supported contract versions are optional to be defined. items: - description: |- - Represents name of the provider with its provider compatibility [contract versions]. - - [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts - properties: - contractVersion: - description: |- - Compatibility restriction in the [CAPI provider format]. The value is an underscore-delimited (_) list of versions. - Optional to be defined. - - [CAPI provider format]: https://cluster-api.sigs.k8s.io/developer/providers/contracts#api-version-labels - type: string - name: - description: Name of the provider. - type: string - type: object + type: string type: array type: object x-kubernetes-validations: @@ -141,13 +129,17 @@ spec: status: description: ProviderTemplateStatus defines the observed state of ProviderTemplate properties: - capiContractVersion: + capiContracts: + additionalProperties: + type: string description: |- - CAPI [contract version] indicating compatibility with the core CAPI. - Currently supported versions: v1alpha3_v1alpha4_v1beta1. + Holds key-value pairs with compatibility [contract versions], + where the key is the core CAPI contract version, + and the value is an underscore-delimited (_) list of provider contract versions + supported by the core CAPI. - [contract version]: https://cluster-api.sigs.k8s.io/developer/providers/contracts - type: string + [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts + type: object chartRef: description: |- ChartRef is a reference to a source controller resource containing the @@ -195,22 +187,7 @@ spec: Providers represent exposed CAPI providers with supported contract versions if the latter has been given. items: - description: |- - Represents name of the provider with its provider compatibility [contract versions]. - - [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts - properties: - contractVersion: - description: |- - Compatibility restriction in the [CAPI provider format]. The value is an underscore-delimited (_) list of versions. - Optional to be defined. - - [CAPI provider format]: https://cluster-api.sigs.k8s.io/developer/providers/contracts#api-version-labels - type: string - name: - description: Name of the provider. - type: string - type: object + type: string type: array valid: description: Valid indicates whether the template passed validation diff --git a/templates/provider/hmc/templates/crds/hmc.mirantis.com_servicetemplates.yaml b/templates/provider/hmc/templates/crds/hmc.mirantis.com_servicetemplates.yaml index 48800b1fe..84c17f57f 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_servicetemplates.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_servicetemplates.yaml @@ -112,22 +112,7 @@ spec: Providers represent requested CAPI providers. Should be set if not present in the Helm chart metadata. items: - description: |- - Represents name of the provider with its provider compatibility [contract versions]. - - [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts - properties: - contractVersion: - description: |- - Compatibility restriction in the [CAPI provider format]. The value is an underscore-delimited (_) list of versions. - Optional to be defined. - - [CAPI provider format]: https://cluster-api.sigs.k8s.io/developer/providers/contracts#api-version-labels - type: string - name: - description: Name of the provider. - type: string - type: object + type: string type: array required: - helm @@ -187,22 +172,7 @@ spec: providers: description: Providers represent requested CAPI providers. items: - description: |- - Represents name of the provider with its provider compatibility [contract versions]. - - [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts - properties: - contractVersion: - description: |- - Compatibility restriction in the [CAPI provider format]. The value is an underscore-delimited (_) list of versions. - Optional to be defined. - - [CAPI provider format]: https://cluster-api.sigs.k8s.io/developer/providers/contracts#api-version-labels - type: string - name: - description: Name of the provider. - type: string - type: object + type: string type: array valid: description: Valid indicates whether the template passed validation diff --git a/templates/provider/k0smotron/Chart.yaml b/templates/provider/k0smotron/Chart.yaml index 9e1e376c2..5db0e9a69 100644 --- a/templates/provider/k0smotron/Chart.yaml +++ b/templates/provider/k0smotron/Chart.yaml @@ -20,4 +20,4 @@ version: 0.0.2 # It is recommended to use it with quotes. appVersion: "1.0.4" annotations: - cluster.x-k8s.io/provider: infrastructure-k0smotron; bootstrap-k0s; control-plane-k0s; control-plane-k0smotron + cluster.x-k8s.io/provider: infrastructure-k0smotron, bootstrap-k0smotron, control-plane-k0smotron diff --git a/test/objects/template/template.go b/test/objects/template/template.go index 9ac4cf412..073be7207 100644 --- a/test/objects/template/template.go +++ b/test/objects/template/template.go @@ -174,13 +174,28 @@ func WithConfigStatus(config string) Opt { } } -func WithProviderStatusCAPIContract(v string) Opt { +func WithProviderStatusCAPIContracts(coreAndProvidersContracts ...string) Opt { + if len(coreAndProvidersContracts)&1 != 0 { + panic("non even number of arguments") + } + return func(template Template) { + if len(coreAndProvidersContracts) == 0 { + return + } + pt, ok := template.(*v1alpha1.ProviderTemplate) if !ok { panic(fmt.Sprintf("unexpected type %T, expected ProviderTemplate", template)) } - pt.Status.CAPIContractVersion = v + + if pt.Status.CAPIContracts == nil { + pt.Status.CAPIContracts = make(v1alpha1.CompatibilityContracts) + } + + for i := 0; i < len(coreAndProvidersContracts)/2; i++ { + pt.Status.CAPIContracts[coreAndProvidersContracts[i*2]] = coreAndProvidersContracts[i*2+1] + } } }