From a3c0cf240da21fcdd8023afa5523062127fc1d00 Mon Sep 17 00:00:00 2001 From: zerospiel Date: Fri, 18 Oct 2024 13:52:18 +0200 Subject: [PATCH 1/3] Align provider names with the CAPI labels * removed nested struct with infra/bstrp/cp providers * removed the corresponding chart annotations * align all providers into a single struct depending on a single annotation with regarding prefixes * migrated provider/cluster templates on the new annotation Closes #497 --- api/v1alpha1/clustertemplate_types.go | 14 +- api/v1alpha1/common.go | 65 ++------ api/v1alpha1/providertemplate_types.go | 14 +- api/v1alpha1/servicetemplate_types.go | 35 +---- api/v1alpha1/templates_common.go | 48 ++---- api/v1alpha1/zz_generated.deepcopy.go | 88 ++++++----- .../controller/managedcluster_controller.go | 95 ++++++------ .../managedcluster_controller_test.go | 16 +- internal/controller/management_controller.go | 8 +- internal/controller/template_controller.go | 16 +- .../controller/template_controller_test.go | 44 +++--- internal/telemetry/event.go | 6 +- internal/telemetry/tracker.go | 5 +- internal/webhook/managedcluster_webhook.go | 37 +++-- .../webhook/managedcluster_webhook_test.go | 50 +++---- internal/webhook/management_webhook_test.go | 2 +- templates/cluster/aws-eks/Chart.yaml | 4 +- templates/cluster/aws-hosted-cp/Chart.yaml | 4 +- .../cluster/aws-standalone-cp/Chart.yaml | 4 +- templates/cluster/azure-hosted-cp/Chart.yaml | 4 +- .../cluster/azure-standalone-cp/Chart.yaml | 4 +- .../cluster/vsphere-hosted-cp/Chart.yaml | 4 +- .../cluster/vsphere-standalone-cp/Chart.yaml | 4 +- .../cluster-api-provider-aws/Chart.yaml | 4 +- .../cluster-api-provider-azure/Chart.yaml | 2 +- .../cluster-api-provider-vsphere/Chart.yaml | 2 +- .../hmc.mirantis.com_clustertemplates.yaml | 140 ++++-------------- .../crds/hmc.mirantis.com_managements.yaml | 70 ++------- .../hmc.mirantis.com_providertemplates.yaml | 140 ++++-------------- .../hmc.mirantis.com_servicetemplates.yaml | 46 +----- templates/provider/k0smotron/Chart.yaml | 4 +- 31 files changed, 308 insertions(+), 671 deletions(-) diff --git a/api/v1alpha1/clustertemplate_types.go b/api/v1alpha1/clustertemplate_types.go index 024cc2d77..e8b211ba8 100644 --- a/api/v1alpha1/clustertemplate_types.go +++ b/api/v1alpha1/clustertemplate_types.go @@ -54,19 +54,9 @@ type ClusterTemplateStatus struct { // either from the spec or from the given annotations. func (t *ClusterTemplate) FillStatusWithProviders(annotations map[string]string) error { var err error - t.Status.Providers.BootstrapProviders, err = parseProviders(t, bootstrapProvidersType, annotations, semver.NewConstraint) + t.Status.Providers, err = parseProviders(t, annotations, semver.NewConstraint) if err != nil { - return fmt.Errorf("failed to parse ClusterTemplate bootstrap providers: %v", err) - } - - t.Status.Providers.ControlPlaneProviders, err = parseProviders(t, controlPlaneProvidersType, annotations, semver.NewConstraint) - if err != nil { - return fmt.Errorf("failed to parse ClusterTemplate controlPlane providers: %v", err) - } - - t.Status.Providers.InfrastructureProviders, err = parseProviders(t, infrastructureProvidersType, annotations, semver.NewConstraint) - if err != nil { - return fmt.Errorf("failed to parse ClusterTemplate infrastructure providers: %v", err) + return fmt.Errorf("failed to parse ClusterTemplate providers: %v", err) } kversion := annotations[ChartAnnotationKubernetesVersion] diff --git a/api/v1alpha1/common.go b/api/v1alpha1/common.go index c4f326b45..fdba2d2f6 100644 --- a/api/v1alpha1/common.go +++ b/api/v1alpha1/common.go @@ -36,30 +36,21 @@ const ( ) type ( + // TODO (zerospiel): unite with the versioned as part of the [Contracts support]. + // + // [Contracts support]: https://github.com/Mirantis/hmc/issues/496 + // Providers hold different types of CAPI providers. - Providers struct { - // InfrastructureProviders is the list of CAPI infrastructure providers - InfrastructureProviders []string `json:"infrastructure,omitempty"` - // BootstrapProviders is the list of CAPI bootstrap providers - BootstrapProviders []string `json:"bootstrap,omitempty"` - // ControlPlaneProviders is the list of CAPI control plane providers - ControlPlaneProviders []string `json:"controlPlane,omitempty"` - } + Providers []string // Holds different types of CAPI providers with either // an exact or constrained version in the SemVer format. The requirement // is determined by a consumer of this type. - ProvidersTupled struct { - // List of CAPI infrastructure providers with either an exact or constrained version in the SemVer format. - // Compatibility attributes are optional to be defined. - InfrastructureProviders []ProviderTuple `json:"infrastructure,omitempty"` - // List of CAPI bootstrap providers with either an exact or constrained version in the SemVer format. - // Compatibility attributes are optional to be defined. - BootstrapProviders []ProviderTuple `json:"bootstrap,omitempty"` - // List of CAPI control plane providers with either an exact or constrained version in the SemVer format. - // Compatibility attributes are optional to be defined. - ControlPlaneProviders []ProviderTuple `json:"controlPlane,omitempty"` - } + + // Holds different types of CAPI providers with either + // an exact or constrained version in the SemVer format. The requirement + // is determined by a consumer of this type. + ProvidersTupled []ProviderTuple // Represents name of the provider with either an exact or constrained version in the SemVer format. ProviderTuple struct { @@ -145,35 +136,11 @@ func ExtractServiceTemplateName(rawObj client.Object) []string { return templates } -func (c ProvidersTupled) BootstrapProvidersNames() []string { - return c.names(bootstrapProvidersType) -} - -func (c ProvidersTupled) ControlPlaneProvidersNames() []string { - return c.names(controlPlaneProvidersType) -} - -func (c ProvidersTupled) InfrastructureProvidersNames() []string { - return c.names(infrastructureProvidersType) -} - -func (c ProvidersTupled) names(typ providersType) []string { - f := func(nn []ProviderTuple) []string { - res := make([]string, len(nn)) - for i, v := range nn { - res[i] = v.Name - } - return res - } - - switch typ { - case bootstrapProvidersType: - return f(c.BootstrapProviders) - case controlPlaneProvidersType: - return f(c.ControlPlaneProviders) - case infrastructureProvidersType: - return f(c.InfrastructureProviders) - default: - return []string{} +// Names flattens the underlaying slice to provider names slice and returns it. +func (c ProvidersTupled) Names() []string { + nn := make([]string, len(c)) + for i, v := range c { + nn[i] = v.Name } + return nn } diff --git a/api/v1alpha1/providertemplate_types.go b/api/v1alpha1/providertemplate_types.go index b71f020cd..ca7ebd6cf 100644 --- a/api/v1alpha1/providertemplate_types.go +++ b/api/v1alpha1/providertemplate_types.go @@ -63,19 +63,9 @@ type ProviderTemplateStatus struct { // either from the spec or from the given annotations. func (t *ProviderTemplate) FillStatusWithProviders(annotations map[string]string) error { var err error - t.Status.Providers.BootstrapProviders, err = parseProviders(t, bootstrapProvidersType, annotations, semver.NewVersion) + t.Status.Providers, err = parseProviders(t, annotations, semver.NewVersion) if err != nil { - return fmt.Errorf("failed to parse ProviderTemplate bootstrap providers: %v", err) - } - - t.Status.Providers.ControlPlaneProviders, err = parseProviders(t, controlPlaneProvidersType, annotations, semver.NewVersion) - if err != nil { - return fmt.Errorf("failed to parse ProviderTemplate controlPlane providers: %v", err) - } - - t.Status.Providers.InfrastructureProviders, err = parseProviders(t, infrastructureProvidersType, annotations, semver.NewVersion) - if err != nil { - return fmt.Errorf("failed to parse ProviderTemplate infrastructure providers: %v", err) + return fmt.Errorf("failed to parse ProviderTemplate providers: %v", err) } if t.Name == CoreCAPIName { diff --git a/api/v1alpha1/servicetemplate_types.go b/api/v1alpha1/servicetemplate_types.go index 540c33eb6..70aaf781d 100644 --- a/api/v1alpha1/servicetemplate_types.go +++ b/api/v1alpha1/servicetemplate_types.go @@ -52,41 +52,20 @@ 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 { - parseProviders := func(typ providersType) []string { - var ( - pspec []string - anno string - ) - switch typ { - case bootstrapProvidersType: - pspec, anno = t.Spec.Providers.BootstrapProviders, ChartAnnotationBootstrapProviders - case controlPlaneProvidersType: - pspec, anno = t.Spec.Providers.ControlPlaneProviders, ChartAnnotationControlPlaneProviders - case infrastructureProvidersType: - pspec, anno = t.Spec.Providers.InfrastructureProviders, ChartAnnotationInfraProviders - } - - providers := annotations[anno] - if len(providers) == 0 { - return pspec - } - + providers := annotations[ChartAnnotationProviderName] + if len(providers) == 0 { + t.Status.Providers = t.Spec.Providers + } else { splitted := strings.Split(providers, multiProviderSeparator) - result := make([]string, 0, len(splitted)) - result = append(result, pspec...) + t.Status.Providers = make([]string, 0, len(splitted)) + t.Status.Providers = append(t.Status.Providers, t.Spec.Providers...) for _, v := range splitted { if c := strings.TrimSpace(v); c != "" { - result = append(result, c) + t.Status.Providers = append(t.Status.Providers, c) } } - - return result } - t.Status.Providers.BootstrapProviders = parseProviders(bootstrapProvidersType) - t.Status.Providers.ControlPlaneProviders = parseProviders(controlPlaneProvidersType) - t.Status.Providers.InfrastructureProviders = parseProviders(infrastructureProvidersType) - kconstraint := annotations[ChartAnnotationKubernetesConstraint] if t.Spec.KubernetesConstraint != "" { kconstraint = t.Spec.KubernetesConstraint diff --git a/api/v1alpha1/templates_common.go b/api/v1alpha1/templates_common.go index 3f1abaf92..688b28e86 100644 --- a/api/v1alpha1/templates_common.go +++ b/api/v1alpha1/templates_common.go @@ -24,12 +24,9 @@ import ( ) const ( - // ChartAnnotationInfraProviders is an annotation containing the CAPI infrastructure providers associated with Template. - ChartAnnotationInfraProviders = "hmc.mirantis.com/infrastructure-providers" - // ChartAnnotationBootstrapProviders is an annotation containing the CAPI bootstrap providers associated with Template. - ChartAnnotationBootstrapProviders = "hmc.mirantis.com/bootstrap-providers" - // ChartAnnotationControlPlaneProviders is an annotation containing the CAPI control plane providers associated with Template. - ChartAnnotationControlPlaneProviders = "hmc.mirantis.com/control-plane-providers" + // 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" ) // +kubebuilder:validation:XValidation:rule="(has(self.chartName) && !has(self.chartRef)) || (!has(self.chartName) && has(self.chartRef))", message="either chartName or chartRef must be set" @@ -77,30 +74,26 @@ type TemplateValidationStatus struct { Valid bool `json:"valid"` } -type providersType int - -const ( - bootstrapProvidersType providersType = iota - controlPlaneProvidersType - infrastructureProvidersType -) - +// TODO (zerospiel): change to comma as part of the [Contracts support]. +// +// [Contracts support]: https://github.com/Mirantis/hmc/issues/496 const multiProviderSeparator = ";" -func parseProviders[T any](providersGetter interface{ GetSpecProviders() ProvidersTupled }, typ providersType, annotations map[string]string, validationFn func(string) (T, error)) ([]ProviderTuple, error) { - pspec, anno := getProvidersSpecAnno(providersGetter, typ) - - providers := annotations[anno] +// TODO (zerospiel): move to the template-ctrl? +func parseProviders[T any](providersGetter interface{ GetSpecProviders() ProvidersTupled }, annotations map[string]string, validationFn func(string) (T, error)) ([]ProviderTuple, error) { + providers := annotations[ChartAnnotationProviderName] if len(providers) == 0 { - return pspec, nil + return providersGetter.GetSpecProviders(), nil } var ( + ps = providersGetter.GetSpecProviders() + splitted = strings.Split(providers, multiProviderSeparator) - pstatus = make([]ProviderTuple, 0, len(splitted)+len(pspec)) + pstatus = make([]ProviderTuple, 0, len(splitted)+len(ps)) merr error ) - pstatus = append(pstatus, pspec...) + pstatus = append(pstatus, ps...) for _, v := range splitted { v = strings.TrimSpace(v) @@ -127,16 +120,3 @@ func parseProviders[T any](providersGetter interface{ GetSpecProviders() Provide return pstatus, merr } - -func getProvidersSpecAnno(providersGetter interface{ GetSpecProviders() ProvidersTupled }, typ providersType) (spec []ProviderTuple, annotation string) { - switch typ { - case bootstrapProvidersType: - return providersGetter.GetSpecProviders().BootstrapProviders, ChartAnnotationBootstrapProviders - case controlPlaneProvidersType: - return providersGetter.GetSpecProviders().ControlPlaneProviders, ChartAnnotationControlPlaneProviders - case infrastructureProvidersType: - return providersGetter.GetSpecProviders().InfrastructureProviders, ChartAnnotationInfraProviders - default: - return []ProviderTuple{}, "" - } -} diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index e14ce1474..706a72b05 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -188,7 +188,11 @@ func (in *ClusterTemplateList) DeepCopyObject() runtime.Object { func (in *ClusterTemplateSpec) DeepCopyInto(out *ClusterTemplateSpec) { *out = *in in.Helm.DeepCopyInto(&out.Helm) - in.Providers.DeepCopyInto(&out.Providers) + if in.Providers != nil { + in, out := &in.Providers, &out.Providers + *out = make(ProvidersTupled, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterTemplateSpec. @@ -204,7 +208,11 @@ 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 - in.Providers.DeepCopyInto(&out.Providers) + if in.Providers != nil { + in, out := &in.Providers, &out.Providers + *out = make(ProvidersTupled, len(*in)) + copy(*out, *in) + } in.TemplateStatusCommon.DeepCopyInto(&out.TemplateStatusCommon) } @@ -603,7 +611,11 @@ func (in *ManagementStatus) DeepCopyInto(out *ManagementStatus) { (*out)[key] = val } } - in.AvailableProviders.DeepCopyInto(&out.AvailableProviders) + if in.AvailableProviders != nil { + in, out := &in.AvailableProviders, &out.AvailableProviders + *out = make(ProvidersTupled, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ManagementStatus. @@ -808,7 +820,11 @@ func (in *ProviderTemplateList) DeepCopyObject() runtime.Object { func (in *ProviderTemplateSpec) DeepCopyInto(out *ProviderTemplateSpec) { *out = *in in.Helm.DeepCopyInto(&out.Helm) - in.Providers.DeepCopyInto(&out.Providers) + if in.Providers != nil { + in, out := &in.Providers, &out.Providers + *out = make(ProvidersTupled, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProviderTemplateSpec. @@ -824,7 +840,11 @@ 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 - in.Providers.DeepCopyInto(&out.Providers) + if in.Providers != nil { + in, out := &in.Providers, &out.Providers + *out = make(ProvidersTupled, len(*in)) + copy(*out, *in) + } in.TemplateStatusCommon.DeepCopyInto(&out.TemplateStatusCommon) } @@ -854,63 +874,41 @@ func (in *ProviderTuple) DeepCopy() *ProviderTuple { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Providers) DeepCopyInto(out *Providers) { - *out = *in - if in.InfrastructureProviders != nil { - in, out := &in.InfrastructureProviders, &out.InfrastructureProviders - *out = make([]string, len(*in)) - copy(*out, *in) - } - if in.BootstrapProviders != nil { - in, out := &in.BootstrapProviders, &out.BootstrapProviders - *out = make([]string, len(*in)) - copy(*out, *in) - } - if in.ControlPlaneProviders != nil { - in, out := &in.ControlPlaneProviders, &out.ControlPlaneProviders - *out = make([]string, len(*in)) +func (in Providers) DeepCopyInto(out *Providers) { + { + in := &in + *out = make(Providers, len(*in)) copy(*out, *in) } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Providers. -func (in *Providers) DeepCopy() *Providers { +func (in Providers) DeepCopy() Providers { if in == nil { return nil } out := new(Providers) in.DeepCopyInto(out) - return out + return *out } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ProvidersTupled) DeepCopyInto(out *ProvidersTupled) { - *out = *in - if in.InfrastructureProviders != nil { - in, out := &in.InfrastructureProviders, &out.InfrastructureProviders - *out = make([]ProviderTuple, len(*in)) - copy(*out, *in) - } - if in.BootstrapProviders != nil { - in, out := &in.BootstrapProviders, &out.BootstrapProviders - *out = make([]ProviderTuple, len(*in)) - copy(*out, *in) - } - if in.ControlPlaneProviders != nil { - in, out := &in.ControlPlaneProviders, &out.ControlPlaneProviders - *out = make([]ProviderTuple, len(*in)) +func (in ProvidersTupled) DeepCopyInto(out *ProvidersTupled) { + { + in := &in + *out = make(ProvidersTupled, len(*in)) copy(*out, *in) } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProvidersTupled. -func (in *ProvidersTupled) DeepCopy() *ProvidersTupled { +func (in ProvidersTupled) DeepCopy() ProvidersTupled { if in == nil { return nil } out := new(ProvidersTupled) in.DeepCopyInto(out) - return out + return *out } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. @@ -1157,7 +1155,11 @@ func (in *ServiceTemplateList) DeepCopyObject() runtime.Object { func (in *ServiceTemplateSpec) DeepCopyInto(out *ServiceTemplateSpec) { *out = *in in.Helm.DeepCopyInto(&out.Helm) - in.Providers.DeepCopyInto(&out.Providers) + if in.Providers != nil { + in, out := &in.Providers, &out.Providers + *out = make(Providers, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ServiceTemplateSpec. @@ -1173,7 +1175,11 @@ func (in *ServiceTemplateSpec) DeepCopy() *ServiceTemplateSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ServiceTemplateStatus) DeepCopyInto(out *ServiceTemplateStatus) { *out = *in - in.Providers.DeepCopyInto(&out.Providers) + if in.Providers != nil { + in, out := &in.Providers, &out.Providers + *out = make(Providers, len(*in)) + copy(*out, *in) + } in.TemplateStatusCommon.DeepCopyInto(&out.TemplateStatusCommon) } diff --git a/internal/controller/managedcluster_controller.go b/internal/controller/managedcluster_controller.go index 1090936d2..121218040 100644 --- a/internal/controller/managedcluster_controller.go +++ b/internal/controller/managedcluster_controller.go @@ -20,6 +20,7 @@ import ( "encoding/json" "errors" "fmt" + "strings" texttemplate "text/template" "time" @@ -69,26 +70,6 @@ type ManagedClusterReconciler struct { SystemNamespace string } -var ( - gvkAWSCluster = schema.GroupVersionKind{ - Group: "infrastructure.cluster.x-k8s.io", - Version: "v1beta2", - Kind: "awscluster", - } - - gvkAzureCluster = schema.GroupVersionKind{ - Group: "infrastructure.cluster.x-k8s.io", - Version: "v1beta1", - Kind: "azurecluster", - } - - gvkMachine = schema.GroupVersionKind{ - Group: "cluster.x-k8s.io", - Version: "v1beta1", - Kind: "machine", - } -) - // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. func (r *ManagedClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ -373,8 +354,8 @@ func (r *ManagedClusterReconciler) Update(ctx context.Context, managedCluster *h return ctrl.Result{RequeueAfter: DefaultRequeueInterval}, nil } - err = r.reconcileCredentialPropagation(ctx, managedCluster) - if err != nil { + if err := r.reconcileCredentialPropagation(ctx, managedCluster); err != nil { + l.Error(err, "failed to reconcile credentials propagation") return ctrl.Result{}, err } @@ -537,17 +518,30 @@ func (r *ManagedClusterReconciler) Delete(ctx context.Context, managedCluster *h } func (r *ManagedClusterReconciler) releaseCluster(ctx context.Context, namespace, name, templateName string) error { - providers, err := r.getProviders(ctx, namespace, templateName) + providers, err := r.getInfraProvidersNames(ctx, namespace, templateName) if err != nil { return err } - for _, provider := range providers.BootstrapProviders { - if provider.Name == "eks" { - // no need to do anything for EKS clusters - return nil + var ( + gvkAWSCluster = schema.GroupVersionKind{ + Group: "infrastructure.cluster.x-k8s.io", + Version: "v1beta2", + Kind: "AWSCluster", + } + + gvkAzureCluster = schema.GroupVersionKind{ + Group: "infrastructure.cluster.x-k8s.io", + Version: "v1beta1", + Kind: "AzureCluster", } - } + + gvkMachine = schema.GroupVersionKind{ + Group: "cluster.x-k8s.io", + Version: "v1beta1", + Kind: "Machine", + } + ) providerGVKs := map[string]schema.GroupVersionKind{ "aws": gvkAWSCluster, @@ -555,14 +549,18 @@ func (r *ManagedClusterReconciler) releaseCluster(ctx context.Context, namespace } // Associate the provider with it's GVK - for _, provider := range providers.InfrastructureProviders { - gvk, ok := providerGVKs[provider.Name] + for _, provider := range providers { + gvk, ok := providerGVKs[provider] if !ok { continue } cluster, err := r.getCluster(ctx, namespace, name, gvk) if err != nil { + if provider == "aws" && apierrors.IsNotFound(err) { + return nil + } + return err } @@ -579,15 +577,22 @@ func (r *ManagedClusterReconciler) releaseCluster(ctx context.Context, namespace return nil } -func (r *ManagedClusterReconciler) getProviders(ctx context.Context, templateNamespace, templateName string) (hmc.ProvidersTupled, error) { +func (r *ManagedClusterReconciler) getInfraProvidersNames(ctx context.Context, templateNamespace, templateName string) ([]string, error) { template := &hmc.ClusterTemplate{} templateRef := client.ObjectKey{Name: templateName, Namespace: templateNamespace} if err := r.Get(ctx, templateRef, template); err != nil { ctrl.LoggerFrom(ctx).Error(err, "Failed to get ClusterTemplate", "template namespace", templateNamespace, "template name", templateName) - return hmc.ProvidersTupled{}, err + return nil, err } - return template.Status.Providers, nil + 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) + } + } + + return ips[:len(ips):len(ips)], nil } func (r *ManagedClusterReconciler) getCluster(ctx context.Context, namespace, name string, gvk schema.GroupVersionKind) (*metav1.PartialObjectMetadata, error) { @@ -638,7 +643,7 @@ func (r *ManagedClusterReconciler) reconcileCredentialPropagation(ctx context.Co l := ctrl.LoggerFrom(ctx) l.Info("Reconciling CCM credentials propagation") - providers, err := r.getProviders(ctx, managedCluster.Namespace, managedCluster.Spec.Template) + providers, err := r.getInfraProvidersNames(ctx, managedCluster.Namespace, managedCluster.Spec.Template) if err != nil { return fmt.Errorf("failed to get cluster providers for cluster %s/%s: %s", managedCluster.Namespace, managedCluster.Name, err) } @@ -651,15 +656,13 @@ func (r *ManagedClusterReconciler) reconcileCredentialPropagation(ctx context.Co return fmt.Errorf("failed to get kubeconfig secret for cluster %s/%s: %s", managedCluster.Namespace, managedCluster.Name, err) } - for _, provider := range providers.InfrastructureProviders { - switch provider.Name { + for _, provider := range providers { + switch provider { case "aws": l.Info("Skipping creds propagation for AWS") - continue case "azure": l.Info("Azure creds propagation start") - err := r.propagateAzureSecrets(ctx, managedCluster, kubeconfSecret) - if err != nil { + if err := r.propagateAzureSecrets(ctx, managedCluster, kubeconfSecret); err != nil { errMsg := fmt.Sprintf("failed to create Azure CCM credentials: %s", err) apimeta.SetStatusCondition(managedCluster.GetConditions(), metav1.Condition{ Type: hmc.CredentialsPropagatedCondition, @@ -667,19 +670,19 @@ func (r *ManagedClusterReconciler) reconcileCredentialPropagation(ctx context.Co Reason: hmc.FailedReason, Message: errMsg, }) + return errors.New(errMsg) } + apimeta.SetStatusCondition(managedCluster.GetConditions(), metav1.Condition{ Type: hmc.CredentialsPropagatedCondition, Status: metav1.ConditionTrue, Reason: hmc.SucceededReason, Message: "Azure CCM credentials created", }) - continue case "vsphere": l.Info("vSphere creds propagation start") - err := r.propagateVSphereSecrets(ctx, managedCluster, kubeconfSecret) - if err != nil { + if err := r.propagateVSphereSecrets(ctx, managedCluster, kubeconfSecret); err != nil { errMsg := fmt.Sprintf("failed to create vSphere CCM credentials: %s", err) apimeta.SetStatusCondition(managedCluster.GetConditions(), metav1.Condition{ Type: hmc.CredentialsPropagatedCondition, @@ -689,25 +692,25 @@ func (r *ManagedClusterReconciler) reconcileCredentialPropagation(ctx context.Co }) return errors.New(errMsg) } + apimeta.SetStatusCondition(managedCluster.GetConditions(), metav1.Condition{ Type: hmc.CredentialsPropagatedCondition, Status: metav1.ConditionTrue, Reason: hmc.SucceededReason, Message: "vSphere CCM credentials created", }) - continue default: - errMsg := fmt.Sprintf("unsupported infrastructure provider %s", provider) apimeta.SetStatusCondition(managedCluster.GetConditions(), metav1.Condition{ Type: hmc.CredentialsPropagatedCondition, Status: metav1.ConditionFalse, Reason: hmc.FailedReason, - Message: errMsg, + Message: fmt.Sprintf("unsupported infrastructure provider %s", provider), }) - continue } } + l.Info("CCM credentials reconcile finished") + return nil } diff --git a/internal/controller/managedcluster_controller_test.go b/internal/controller/managedcluster_controller_test.go index a7070fa7e..029b544a9 100644 --- a/internal/controller/managedcluster_controller_test.go +++ b/internal/controller/managedcluster_controller_test.go @@ -94,13 +94,7 @@ var _ = Describe("ManagedCluster Controller", func() { Raw: []byte(`{"foo":"bar"}`), }, }, - Providers: hmc.ProvidersTupled{ - InfrastructureProviders: []hmc.ProviderTuple{ - { - Name: "aws", - }, - }, - }, + Providers: hmc.ProvidersTupled{{Name: "infrastructure-aws"}}, } Expect(k8sClient.Status().Update(ctx, template)).To(Succeed()) } @@ -116,13 +110,7 @@ var _ = Describe("ManagedCluster Controller", func() { } Expect(k8sClient.Create(ctx, management)).To(Succeed()) management.Status = hmc.ManagementStatus{ - AvailableProviders: hmc.ProvidersTupled{ - InfrastructureProviders: []hmc.ProviderTuple{ - { - Name: "aws", - }, - }, - }, + AvailableProviders: hmc.ProvidersTupled{{Name: "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 41a1f6723..537dc1e43 100644 --- a/internal/controller/management_controller.go +++ b/internal/controller/management_controller.go @@ -19,6 +19,8 @@ import ( "encoding/json" "errors" "fmt" + "slices" + "strings" fluxv2 "github.com/fluxcd/helm-controller/api/v2" "github.com/fluxcd/pkg/apis/meta" @@ -425,9 +427,9 @@ func updateComponentsStatus( } if err == "" { - providers.InfrastructureProviders = append(providers.InfrastructureProviders, templateProviders.InfrastructureProviders...) - providers.BootstrapProviders = append(providers.BootstrapProviders, templateProviders.BootstrapProviders...) - providers.ControlPlaneProviders = append(providers.ControlPlaneProviders, templateProviders.ControlPlaneProviders...) + *providers = append(*providers, templateProviders...) + slices.SortFunc(*providers, func(a, b hmc.ProviderTuple) int { return strings.Compare(a.Name, b.Name) }) + *providers = slices.Compact(*providers) } } diff --git a/internal/controller/template_controller.go b/internal/controller/template_controller.go index 6611a1901..9e0c403a9 100644 --- a/internal/controller/template_controller.go +++ b/internal/controller/template_controller.go @@ -344,13 +344,7 @@ func (r *ClusterTemplateReconciler) validateCompatibilityAttrs(ctx context.Conte ctrl.LoggerFrom(ctx).V(1).Info("providers to check", "exposed", exposedProviders, "required", requiredProviders) var merr error - missing, wrong, parsing := collectMissingProvidersWithWrongVersions("bootstrap", exposedProviders.BootstrapProviders, requiredProviders.BootstrapProviders) - merr = errors.Join(merr, missing, wrong, parsing) - - missing, wrong, parsing = collectMissingProvidersWithWrongVersions("control plane", exposedProviders.ControlPlaneProviders, requiredProviders.ControlPlaneProviders) - merr = errors.Join(merr, missing, wrong, parsing) - - missing, wrong, parsing = collectMissingProvidersWithWrongVersions("infrastructure", exposedProviders.InfrastructureProviders, requiredProviders.InfrastructureProviders) + missing, wrong, parsing := collectMissingProvidersWithWrongVersions(exposedProviders, requiredProviders) merr = errors.Join(merr, missing, wrong, parsing) if merr != nil { @@ -363,7 +357,7 @@ func (r *ClusterTemplateReconciler) validateCompatibilityAttrs(ctx context.Conte // collectMissingProvidersWithWrongVersions returns collected errors for missing providers, providers with // wrong versions that do not satisfy the corresponding constraints, and parsing errors respectevly. -func collectMissingProvidersWithWrongVersions(typ string, exposed, required []hmc.ProviderTuple) (missingErr, nonSatisfyingErr, parsingErr error) { +func collectMissingProvidersWithWrongVersions(exposed, required []hmc.ProviderTuple) (missingErr, nonSatisfyingErr, parsingErr error) { exposedSet := make(map[string]hmc.ProviderTuple, len(exposed)) for _, v := range exposed { exposedSet[v.Name] = v @@ -403,16 +397,16 @@ func collectMissingProvidersWithWrongVersions(typ string, exposed, required []hm if len(missing) > 0 { slices.Sort(missing) - missingErr = fmt.Errorf("one or more required %s providers are not deployed yet: %v", typ, missing) + missingErr = 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 %s providers does not satisfy constraints: %v", typ, nonSatisfying) + nonSatisfyingErr = fmt.Errorf("one or more required providers does not satisfy constraints: %v", nonSatisfying) } if parsingErr != nil { - parsingErr = fmt.Errorf("one or more errors parsing %s providers' versions and constraints : %v", typ, parsingErr) + parsingErr = fmt.Errorf("one or more errors parsing providers' versions and constraints : %v", parsingErr) } return missingErr, nonSatisfyingErr, parsingErr diff --git a/internal/controller/template_controller_test.go b/internal/controller/template_controller_test.go index 3fec09bbc..fe6cf6d4a 100644 --- a/internal/controller/template_controller_test.go +++ b/internal/controller/template_controller_test.go @@ -217,12 +217,10 @@ var _ = Describe("Template Controller", func() { }, Spec: hmcmirantiscomv1alpha1.ClusterTemplateSpec{ Helm: helmSpec, - Providers: hmcmirantiscomv1alpha1.ProvidersTupled{ - BootstrapProviders: []hmcmirantiscomv1alpha1.ProviderTuple{ - { - Name: someProviderName, - VersionOrConstraint: someProviderVersionConstraint, // constraint - }, + Providers: []hmcmirantiscomv1alpha1.ProviderTuple{ + { + Name: someProviderName, + VersionOrConstraint: someProviderVersionConstraint, // constraint }, }, }, @@ -235,12 +233,12 @@ var _ = Describe("Template Controller", func() { return err } - if l := len(clusterTemplate.Spec.Providers.BootstrapProviders); l != 1 { - return fmt.Errorf("expected .spec.providers.bootstrapProviders length to be exactly 1, got %d", l) + if l := len(clusterTemplate.Spec.Providers); l != 1 { + return fmt.Errorf("expected .spec.providers length to be exactly 1, got %d", l) } - if v := clusterTemplate.Spec.Providers.BootstrapProviders[0]; v.Name != someProviderName || v.VersionOrConstraint != someProviderVersionConstraint { - return fmt.Errorf("expected .spec.providers.bootstrapProviders[0] to be %s:%s, got %s:%s", someProviderName, someProviderVersionConstraint, v.Name, v.VersionOrConstraint) + if v := clusterTemplate.Spec.Providers[0]; v.Name != someProviderName || v.VersionOrConstraint != someProviderVersionConstraint { + return fmt.Errorf("expected .spec.providers[0] to be %s:%s, got %s:%s", someProviderName, someProviderVersionConstraint, v.Name, v.VersionOrConstraint) } return nil @@ -255,12 +253,10 @@ var _ = Describe("Template Controller", func() { } Expect(k8sClient.Create(ctx, mgmt)).To(Succeed()) mgmt.Status = hmcmirantiscomv1alpha1.ManagementStatus{ - AvailableProviders: hmcmirantiscomv1alpha1.ProvidersTupled{ - BootstrapProviders: []hmcmirantiscomv1alpha1.ProviderTuple{ - { - Name: someProviderName, - VersionOrConstraint: someProviderVersion, // version - }, + AvailableProviders: []hmcmirantiscomv1alpha1.ProviderTuple{ + { + Name: someProviderName, + VersionOrConstraint: someProviderVersion, // version }, }, } @@ -272,16 +268,12 @@ var _ = Describe("Template Controller", func() { return err } - if l := len(mgmt.Status.AvailableProviders.BootstrapProviders); l != 1 { - return fmt.Errorf("expected .status.availableProviders.bootstrapProviders length to be exactly 1, got %d", l) - } - - if l := len(mgmt.Status.AvailableProviders.BootstrapProviders); l != 1 { - return fmt.Errorf("expected .status.availableProviders.bootstrapProviders length to be exactly 1, got %d", l) + if l := len(mgmt.Status.AvailableProviders); l != 1 { + return fmt.Errorf("expected .status.availableProviders length to be exactly 1, got %d", l) } - if v := mgmt.Status.AvailableProviders.BootstrapProviders[0]; v.Name != someProviderName || v.VersionOrConstraint != someProviderVersion { - return fmt.Errorf("expected .status.availableProviders.bootstrapProviders[0] to be %s:%s, got %s:%s", someProviderName, someProviderVersionConstraint, v.Name, v.VersionOrConstraint) + if v := mgmt.Status.AvailableProviders[0]; v.Name != someProviderName || v.VersionOrConstraint != someProviderVersion { + return fmt.Errorf("expected .status.availableProviders[0] to be %s:%s, got %s:%s", someProviderName, someProviderVersionConstraint, v.Name, v.VersionOrConstraint) } return nil @@ -301,8 +293,8 @@ var _ = Describe("Template Controller", func() { By("Having the valid cluster template status") Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(clusterTemplate), clusterTemplate)).To(Succeed()) Expect(clusterTemplate.Status.Valid && clusterTemplate.Status.ValidationError == "").To(BeTrue()) - Expect(clusterTemplate.Status.Providers.BootstrapProviders).To(HaveLen(1)) - Expect(clusterTemplate.Status.Providers.BootstrapProviders[0]).To(Equal(hmcmirantiscomv1alpha1.ProviderTuple{Name: someProviderName, VersionOrConstraint: someProviderVersionConstraint})) + Expect(clusterTemplate.Status.Providers).To(HaveLen(1)) + Expect(clusterTemplate.Status.Providers[0]).To(Equal(hmcmirantiscomv1alpha1.ProviderTuple{Name: someProviderName, VersionOrConstraint: someProviderVersionConstraint})) By("Removing the created objects") Expect(k8sClient.Delete(ctx, mgmt)).To(Succeed()) diff --git a/internal/telemetry/event.go b/internal/telemetry/event.go index 176470905..81dd10971 100644 --- a/internal/telemetry/event.go +++ b/internal/telemetry/event.go @@ -35,16 +35,14 @@ func TrackManagedClusterCreate(id, managedClusterID, template string, dryRun boo return TrackEvent(managedClusterCreateEvent, id, props) } -func TrackManagedClusterHeartbeat(id, managedClusterID, clusterID, template, templateHelmChartVersion, infrastructureProvider, bootstrapProvider, controlPlaneProvider string) error { +func TrackManagedClusterHeartbeat(id, managedClusterID, clusterID, template, templateHelmChartVersion string, providers []string) error { props := map[string]any{ "hmcVersion": build.Version, "managedClusterID": managedClusterID, "clusterID": clusterID, "template": template, "templateHelmChartVersion": templateHelmChartVersion, - "infrastructureProvider": infrastructureProvider, - "bootstrapProvider": bootstrapProvider, - "controlPlaneProvider": controlPlaneProvider, + "providers": providers, } return TrackEvent(managedClusterHeartbeatEvent, id, props) } diff --git a/internal/telemetry/tracker.go b/internal/telemetry/tracker.go index 83d9b858e..8621dd23a 100644 --- a/internal/telemetry/tracker.go +++ b/internal/telemetry/tracker.go @@ -18,7 +18,6 @@ import ( "context" "errors" "fmt" - "strings" "time" "sigs.k8s.io/controller-runtime/pkg/client" @@ -93,9 +92,7 @@ func (t *Tracker) trackManagedClusterHeartbeat(ctx context.Context) error { clusterID, managedCluster.Spec.Template, template.Spec.Helm.ChartVersion, - strings.Join(template.Status.Providers.InfrastructureProvidersNames(), ","), - strings.Join(template.Status.Providers.BootstrapProvidersNames(), ","), - strings.Join(template.Status.Providers.ControlPlaneProvidersNames(), ","), + template.Status.Providers.Names(), ) 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 18b66b0eb..6884f7416 100644 --- a/internal/webhook/managedcluster_webhook.go +++ b/internal/webhook/managedcluster_webhook.go @@ -17,6 +17,7 @@ package webhook import ( "context" "fmt" + "strings" "github.com/Masterminds/semver/v3" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -212,9 +213,19 @@ func isTemplateValid(template *hmcv1alpha1.ClusterTemplate) error { } func (v *ManagedClusterValidator) validateCredential(ctx context.Context, managedCluster *hmcv1alpha1.ManagedCluster, template *hmcv1alpha1.ClusterTemplate) error { - infraProviders := template.Status.Providers.InfrastructureProviders + if len(template.Status.Providers) == 0 { + return fmt.Errorf("template %q has no providers defined", template.Name) + } + + hasInfra := false + for _, v := range template.Status.Providers { + if strings.HasPrefix(v.Name, "infrastructure-") { + hasInfra = true + break + } + } - if len(infraProviders) == 0 { + if !hasInfra { return fmt.Errorf("template %q has no infrastructure providers defined", template.Name) } @@ -232,31 +243,33 @@ func (v *ManagedClusterValidator) validateCredential(ctx context.Context, manage func isCredMatchTemplate(cred *hmcv1alpha1.Credential, template *hmcv1alpha1.ClusterTemplate) error { idtyKind := cred.Spec.IdentityRef.Kind - infraProviders := template.Status.Providers.InfrastructureProviders - errMsg := func(idtyKind string, provider string) error { + errMsg := func(provider string) error { return fmt.Errorf("wrong kind of the ClusterIdentity %q for provider %q", idtyKind, provider) } - for _, provider := range infraProviders { + for _, provider := range template.Status.Providers { switch provider.Name { - case "aws": + case "infrastructure-aws": if idtyKind != "AWSClusterStaticIdentity" && idtyKind != "AWSClusterRoleIdentity" && idtyKind != "AWSClusterControllerIdentity" { - return errMsg(idtyKind, provider.Name) + return errMsg(provider.Name) } - case "azure": + case "infrastructure-azure": if idtyKind != "AzureClusterIdentity" { - return errMsg(idtyKind, provider.Name) + return errMsg(provider.Name) } - case "vsphere": + case "infrastructure-vsphere": if idtyKind != "VSphereClusterIdentity" { - return errMsg(idtyKind, provider.Name) + return errMsg(provider.Name) } default: - return fmt.Errorf("unsupported infrastructure provider %s", provider.Name) + if strings.HasPrefix(provider.Name, "infrastructure-") { + return fmt.Errorf("unsupported infrastructure provider %s", provider.Name) + } } } + return nil } diff --git a/internal/webhook/managedcluster_webhook_test.go b/internal/webhook/managedcluster_webhook_test.go index 4bd961898..ad8261de6 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.ProvidersTupled{ - InfrastructureProviders: []v1alpha1.ProviderTuple{{Name: "aws"}}, - BootstrapProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, - ControlPlaneProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + {Name: "infrastructure-aws"}, + {Name: "control-plane-k0s"}, + {Name: "bootstrap-k0s"}, }), ) @@ -128,9 +128,9 @@ func TestManagedClusterValidateCreate(t *testing.T) { template.NewClusterTemplate( template.WithName(testTemplateName), template.WithProvidersStatus(v1alpha1.ProvidersTupled{ - InfrastructureProviders: []v1alpha1.ProviderTuple{{Name: "aws"}}, - BootstrapProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, - ControlPlaneProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + {Name: "infrastructure-aws"}, + {Name: "control-plane-k0s"}, + {Name: "bootstrap-k0s"}, }), template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), ), @@ -145,9 +145,9 @@ func TestManagedClusterValidateCreate(t *testing.T) { existingObjects: []runtime.Object{ cred, management.NewManagement(management.WithAvailableProviders(v1alpha1.ProvidersTupled{ - InfrastructureProviders: []v1alpha1.ProviderTuple{{Name: "aws", VersionOrConstraint: "v1.0.0"}}, - BootstrapProviders: []v1alpha1.ProviderTuple{{Name: "k0s", VersionOrConstraint: "v1.0.0"}}, - ControlPlaneProviders: []v1alpha1.ProviderTuple{{Name: "k0s", VersionOrConstraint: "v1.0.0"}}, + {Name: "infrastructure-aws", VersionOrConstraint: "v1.0.0"}, + {Name: "control-plane-k0s", VersionOrConstraint: "v1.0.0"}, + {Name: "bootstrap-k0s", VersionOrConstraint: "v1.0.0"}, })), template.NewClusterTemplate( template.WithName(testTemplateName), @@ -171,9 +171,9 @@ func TestManagedClusterValidateCreate(t *testing.T) { template.NewClusterTemplate( template.WithName(testTemplateName), template.WithProvidersStatus(v1alpha1.ProvidersTupled{ - InfrastructureProviders: []v1alpha1.ProviderTuple{{Name: "aws"}}, - BootstrapProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, - ControlPlaneProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + {Name: "infrastructure-aws"}, + {Name: "control-plane-k0s"}, + {Name: "bootstrap-k0s"}, }), template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), ), @@ -200,9 +200,9 @@ func TestManagedClusterValidateCreate(t *testing.T) { template.NewClusterTemplate( template.WithName(testTemplateName), template.WithProvidersStatus(v1alpha1.ProvidersTupled{ - InfrastructureProviders: []v1alpha1.ProviderTuple{{Name: "aws"}}, - BootstrapProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, - ControlPlaneProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + {Name: "infrastructure-aws"}, + {Name: "control-plane-k0s"}, + {Name: "bootstrap-k0s"}, }), template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), ), @@ -219,22 +219,22 @@ func TestManagedClusterValidateCreate(t *testing.T) { cred, management.NewManagement( management.WithAvailableProviders(v1alpha1.ProvidersTupled{ - InfrastructureProviders: []v1alpha1.ProviderTuple{{Name: "azure"}}, - BootstrapProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, - ControlPlaneProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + {Name: "infrastructure-azure"}, + {Name: "control-plane-k0s"}, + {Name: "bootstrap-k0s"}, }), ), template.NewClusterTemplate( template.WithName(testTemplateName), template.WithProvidersStatus(v1alpha1.ProvidersTupled{ - InfrastructureProviders: []v1alpha1.ProviderTuple{{Name: "azure"}}, - BootstrapProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, - ControlPlaneProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + {Name: "infrastructure-azure"}, + {Name: "control-plane-k0s"}, + {Name: "bootstrap-k0s"}, }), template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), ), }, - err: "the ManagedCluster is invalid: wrong kind of the ClusterIdentity \"AWSClusterStaticIdentity\" for provider \"azure\"", + err: "the ManagedCluster is invalid: wrong kind of the ClusterIdentity \"AWSClusterStaticIdentity\" for provider \"infrastructure-azure\"", }, } for _, tt := range tests { @@ -311,9 +311,9 @@ func TestManagedClusterValidateUpdate(t *testing.T) { ValidationError: "validation error example", }), template.WithProvidersStatus(v1alpha1.ProvidersTupled{ - InfrastructureProviders: []v1alpha1.ProviderTuple{{Name: "aws"}}, - BootstrapProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, - ControlPlaneProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + {Name: "infrastructure-aws"}, + {Name: "control-plane-k0s"}, + {Name: "bootstrap-k0s"}, }), ), }, diff --git a/internal/webhook/management_webhook_test.go b/internal/webhook/management_webhook_test.go index 13bb2942d..a3d311475 100644 --- a/internal/webhook/management_webhook_test.go +++ b/internal/webhook/management_webhook_test.go @@ -46,7 +46,7 @@ func TestManagementValidateUpdate(t *testing.T) { ) providerAwsDefaultTpl := v1alpha1.Provider{ - Name: "aws", + Name: "infrastructure-aws", Component: v1alpha1.Component{ Template: template.DefaultName, }, diff --git a/templates/cluster/aws-eks/Chart.yaml b/templates/cluster/aws-eks/Chart.yaml index 80a86ecea..610a5cc4f 100644 --- a/templates/cluster/aws-eks/Chart.yaml +++ b/templates/cluster/aws-eks/Chart.yaml @@ -8,6 +8,4 @@ type: application # Versions are expected to follow Semantic Versioning (https://semver.org/) version: 0.0.2 annotations: - hmc.mirantis.com/infrastructure-providers: aws - hmc.mirantis.com/controlplane-providers: eks - hmc.mirantis.com/bootstrap-providers: eks + cluster.x-k8s.io/provider: infrastructure-aws diff --git a/templates/cluster/aws-hosted-cp/Chart.yaml b/templates/cluster/aws-hosted-cp/Chart.yaml index 64f6d8f68..10c1fc142 100644 --- a/templates/cluster/aws-hosted-cp/Chart.yaml +++ b/templates/cluster/aws-hosted-cp/Chart.yaml @@ -14,6 +14,4 @@ version: 0.0.2 # It is recommended to use it with quotes. appVersion: "v1.31.1+k0s.1" annotations: - hmc.mirantis.com/infrastructure-providers: aws - hmc.mirantis.com/control-plane-providers: k0smotron - hmc.mirantis.com/bootstrap-providers: k0s + cluster.x-k8s.io/provider: infrastructure-aws; control-plane-k0smotron; bootstrap-k0s diff --git a/templates/cluster/aws-standalone-cp/Chart.yaml b/templates/cluster/aws-standalone-cp/Chart.yaml index 01f0fcada..93edbe18e 100644 --- a/templates/cluster/aws-standalone-cp/Chart.yaml +++ b/templates/cluster/aws-standalone-cp/Chart.yaml @@ -13,6 +13,4 @@ version: 0.0.2 # It is recommended to use it with quotes. appVersion: "v1.31.1+k0s.1" annotations: - hmc.mirantis.com/infrastructure-providers: aws - hmc.mirantis.com/control-plane-providers: k0s - hmc.mirantis.com/bootstrap-providers: k0s + cluster.x-k8s.io/provider: infrastructure-aws; control-plane-k0s; bootstrap-k0s diff --git a/templates/cluster/azure-hosted-cp/Chart.yaml b/templates/cluster/azure-hosted-cp/Chart.yaml index 36cc4a85f..46dcca48f 100644 --- a/templates/cluster/azure-hosted-cp/Chart.yaml +++ b/templates/cluster/azure-hosted-cp/Chart.yaml @@ -14,6 +14,4 @@ version: 0.0.3 # It is recommended to use it with quotes. appVersion: "v1.31.1+k0s.1" annotations: - hmc.mirantis.com/infrastructure-providers: azure - hmc.mirantis.com/control-plane-providers: k0s - hmc.mirantis.com/bootstrap-providers: k0s + cluster.x-k8s.io/provider: infrastructure-azure; control-plane-k0s; bootstrap-k0s diff --git a/templates/cluster/azure-standalone-cp/Chart.yaml b/templates/cluster/azure-standalone-cp/Chart.yaml index c8c90e5c1..e094bfea0 100644 --- a/templates/cluster/azure-standalone-cp/Chart.yaml +++ b/templates/cluster/azure-standalone-cp/Chart.yaml @@ -13,6 +13,4 @@ version: 0.0.3 # It is recommended to use it with quotes. appVersion: "1.31.1+k0s.1" annotations: - hmc.mirantis.com/infrastructure-providers: azure - hmc.mirantis.com/control-plane-providers: k0s - hmc.mirantis.com/bootstrap-providers: k0s + cluster.x-k8s.io/provider: infrastructure-azure; control-plane-k0s; bootstrap-k0s diff --git a/templates/cluster/vsphere-hosted-cp/Chart.yaml b/templates/cluster/vsphere-hosted-cp/Chart.yaml index 63c943e5e..e33bb4d3c 100644 --- a/templates/cluster/vsphere-hosted-cp/Chart.yaml +++ b/templates/cluster/vsphere-hosted-cp/Chart.yaml @@ -14,7 +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 hmc.mirantis.com/type: deployment - hmc.mirantis.com/infrastructure-providers: vsphere - hmc.mirantis.com/control-plane-providers: k0s - hmc.mirantis.com/bootstrap-providers: k0s diff --git a/templates/cluster/vsphere-standalone-cp/Chart.yaml b/templates/cluster/vsphere-standalone-cp/Chart.yaml index c96eadd20..49b29322f 100644 --- a/templates/cluster/vsphere-standalone-cp/Chart.yaml +++ b/templates/cluster/vsphere-standalone-cp/Chart.yaml @@ -13,7 +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 hmc.mirantis.com/type: deployment - hmc.mirantis.com/infrastructure-providers: vsphere - hmc.mirantis.com/control-plane-providers: k0s - hmc.mirantis.com/bootstrap-providers: k0s diff --git a/templates/provider/cluster-api-provider-aws/Chart.yaml b/templates/provider/cluster-api-provider-aws/Chart.yaml index b3b3ab54f..86821d729 100644 --- a/templates/provider/cluster-api-provider-aws/Chart.yaml +++ b/templates/provider/cluster-api-provider-aws/Chart.yaml @@ -20,6 +20,4 @@ version: 0.0.2 # It is recommended to use it with quotes. appVersion: "2.6.1" annotations: - hmc.mirantis.com/infrastructure-providers: aws - hmc.mirantis.com/controlplane-providers: eks - hmc.mirantis.com/bootstrap-providers: eks + cluster.x-k8s.io/provider: infrastructure-aws diff --git a/templates/provider/cluster-api-provider-azure/Chart.yaml b/templates/provider/cluster-api-provider-azure/Chart.yaml index 172b7eee3..02feff0d7 100644 --- a/templates/provider/cluster-api-provider-azure/Chart.yaml +++ b/templates/provider/cluster-api-provider-azure/Chart.yaml @@ -20,4 +20,4 @@ version: 0.0.2 # It is recommended to use it with quotes. appVersion: "1.17.0" annotations: - hmc.mirantis.com/infrastructure-providers: azure + cluster.x-k8s.io/provider: infrastructure-azure diff --git a/templates/provider/cluster-api-provider-vsphere/Chart.yaml b/templates/provider/cluster-api-provider-vsphere/Chart.yaml index 4a4db32ab..ce9008714 100644 --- a/templates/provider/cluster-api-provider-vsphere/Chart.yaml +++ b/templates/provider/cluster-api-provider-vsphere/Chart.yaml @@ -20,4 +20,4 @@ version: 0.0.2 # It is recommended to use it with quotes. appVersion: "1.11.1" annotations: - hmc.mirantis.com/infrastructure-providers: vsphere + cluster.x-k8s.io/provider: infrastructure-vsphere 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 51fc2e84e..bd386db7d 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_clustertemplates.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_clustertemplates.yaml @@ -112,62 +112,20 @@ spec: Providers represent required CAPI providers with constrained compatibility versions set. Should be set if not present in the Helm chart metadata. Compatibility attributes are optional to be defined. - properties: - bootstrap: - description: |- - List of CAPI bootstrap providers with either an exact or constrained version in the SemVer format. - Compatibility attributes are optional to be defined. - items: - description: Represents name of the provider with either an - exact or constrained version in the SemVer format. - properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: - description: |- - Compatibility restriction in the SemVer format (exact or constrained version). - Optional to be defined. - type: string - type: object - type: array - controlPlane: - description: |- - List of CAPI control plane providers with either an exact or constrained version in the SemVer format. - Compatibility attributes are optional to be defined. - items: - description: Represents name of the provider with either an - exact or constrained version in the SemVer format. - properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: - description: |- - Compatibility restriction in the SemVer format (exact or constrained version). - Optional to be defined. - type: string - type: object - type: array - infrastructure: - description: |- - List of CAPI infrastructure providers with either an exact or constrained version in the SemVer format. - Compatibility attributes are optional to be defined. - items: - description: Represents name of the provider with either an - exact or constrained version in the SemVer format. - properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: - description: |- - Compatibility restriction in the SemVer format (exact or constrained version). - Optional to be defined. - type: string - type: object - type: array - type: object + items: + description: Represents name of the provider with either an exact + or constrained version in the SemVer format. + properties: + name: + description: Name of the provider. + type: string + versionOrConstraint: + description: |- + Compatibility restriction in the SemVer format (exact or constrained version). + Optional to be defined. + type: string + type: object + type: array required: - helm type: object @@ -227,62 +185,20 @@ spec: description: |- Providers represent required CAPI providers with constrained compatibility versions set if the latter has been given. - properties: - bootstrap: - description: |- - List of CAPI bootstrap providers with either an exact or constrained version in the SemVer format. - Compatibility attributes are optional to be defined. - items: - description: Represents name of the provider with either an - exact or constrained version in the SemVer format. - properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: - description: |- - Compatibility restriction in the SemVer format (exact or constrained version). - Optional to be defined. - type: string - type: object - type: array - controlPlane: - description: |- - List of CAPI control plane providers with either an exact or constrained version in the SemVer format. - Compatibility attributes are optional to be defined. - items: - description: Represents name of the provider with either an - exact or constrained version in the SemVer format. - properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: - description: |- - Compatibility restriction in the SemVer format (exact or constrained version). - Optional to be defined. - type: string - type: object - type: array - infrastructure: - description: |- - List of CAPI infrastructure providers with either an exact or constrained version in the SemVer format. - Compatibility attributes are optional to be defined. - items: - description: Represents name of the provider with either an - exact or constrained version in the SemVer format. - properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: - description: |- - Compatibility restriction in the SemVer format (exact or constrained version). - Optional to be defined. - type: string - type: object - type: array - type: object + items: + description: Represents name of the provider with either an exact + or constrained version in the SemVer format. + properties: + name: + description: Name of the provider. + type: string + versionOrConstraint: + description: |- + Compatibility restriction in the SemVer format (exact or constrained version). + Optional to be defined. + type: string + type: object + type: array valid: description: Valid indicates whether the template passed validation or not. 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 8ffcde13e..d69574975 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_managements.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_managements.yaml @@ -115,62 +115,20 @@ spec: description: |- AvailableProviders holds all CAPI providers available along with their exact compatibility versions if specified in ProviderTemplates on the Management cluster. - properties: - bootstrap: - description: |- - List of CAPI bootstrap providers with either an exact or constrained version in the SemVer format. - Compatibility attributes are optional to be defined. - items: - description: Represents name of the provider with either an - exact or constrained version in the SemVer format. - properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: - description: |- - Compatibility restriction in the SemVer format (exact or constrained version). - Optional to be defined. - type: string - type: object - type: array - controlPlane: - description: |- - List of CAPI control plane providers with either an exact or constrained version in the SemVer format. - Compatibility attributes are optional to be defined. - items: - description: Represents name of the provider with either an - exact or constrained version in the SemVer format. - properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: - description: |- - Compatibility restriction in the SemVer format (exact or constrained version). - Optional to be defined. - type: string - type: object - type: array - infrastructure: - description: |- - List of CAPI infrastructure providers with either an exact or constrained version in the SemVer format. - Compatibility attributes are optional to be defined. - items: - description: Represents name of the provider with either an - exact or constrained version in the SemVer format. - properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: - description: |- - Compatibility restriction in the SemVer format (exact or constrained version). - Optional to be defined. - type: string - type: object - type: array - type: object + items: + description: Represents name of the provider with either an exact + or constrained version in the SemVer format. + properties: + name: + description: Name of the provider. + type: string + versionOrConstraint: + description: |- + Compatibility restriction in the SemVer format (exact or constrained version). + Optional to be defined. + type: string + type: object + type: array 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 39ddece49..1e584155f 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_providertemplates.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_providertemplates.yaml @@ -118,62 +118,20 @@ spec: Providers represent exposed CAPI providers with exact compatibility versions set. Should be set if not present in the Helm chart metadata. Compatibility attributes are optional to be defined. - properties: - bootstrap: - description: |- - List of CAPI bootstrap providers with either an exact or constrained version in the SemVer format. - Compatibility attributes are optional to be defined. - items: - description: Represents name of the provider with either an - exact or constrained version in the SemVer format. - properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: - description: |- - Compatibility restriction in the SemVer format (exact or constrained version). - Optional to be defined. - type: string - type: object - type: array - controlPlane: - description: |- - List of CAPI control plane providers with either an exact or constrained version in the SemVer format. - Compatibility attributes are optional to be defined. - items: - description: Represents name of the provider with either an - exact or constrained version in the SemVer format. - properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: - description: |- - Compatibility restriction in the SemVer format (exact or constrained version). - Optional to be defined. - type: string - type: object - type: array - infrastructure: - description: |- - List of CAPI infrastructure providers with either an exact or constrained version in the SemVer format. - Compatibility attributes are optional to be defined. - items: - description: Represents name of the provider with either an - exact or constrained version in the SemVer format. - properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: - description: |- - Compatibility restriction in the SemVer format (exact or constrained version). - Optional to be defined. - type: string - type: object - type: array - type: object + items: + description: Represents name of the provider with either an exact + or constrained version in the SemVer format. + properties: + name: + description: Name of the provider. + type: string + versionOrConstraint: + description: |- + Compatibility restriction in the SemVer format (exact or constrained version). + Optional to be defined. + type: string + type: object + type: array type: object x-kubernetes-validations: - message: Spec is immutable @@ -239,62 +197,20 @@ spec: description: |- Providers represent exposed CAPI providers with exact compatibility versions set if the latter has been given. - properties: - bootstrap: - description: |- - List of CAPI bootstrap providers with either an exact or constrained version in the SemVer format. - Compatibility attributes are optional to be defined. - items: - description: Represents name of the provider with either an - exact or constrained version in the SemVer format. - properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: - description: |- - Compatibility restriction in the SemVer format (exact or constrained version). - Optional to be defined. - type: string - type: object - type: array - controlPlane: - description: |- - List of CAPI control plane providers with either an exact or constrained version in the SemVer format. - Compatibility attributes are optional to be defined. - items: - description: Represents name of the provider with either an - exact or constrained version in the SemVer format. - properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: - description: |- - Compatibility restriction in the SemVer format (exact or constrained version). - Optional to be defined. - type: string - type: object - type: array - infrastructure: - description: |- - List of CAPI infrastructure providers with either an exact or constrained version in the SemVer format. - Compatibility attributes are optional to be defined. - items: - description: Represents name of the provider with either an - exact or constrained version in the SemVer format. - properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: - description: |- - Compatibility restriction in the SemVer format (exact or constrained version). - Optional to be defined. - type: string - type: object - type: array - type: object + items: + description: Represents name of the provider with either an exact + or constrained version in the SemVer format. + properties: + name: + description: Name of the provider. + type: string + versionOrConstraint: + description: |- + Compatibility restriction in the SemVer format (exact or constrained version). + Optional to be defined. + type: string + type: object + type: array valid: description: Valid indicates whether the template passed validation or not. 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 967e71ac7..84c17f57f 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_servicetemplates.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_servicetemplates.yaml @@ -111,26 +111,9 @@ spec: description: |- Providers represent requested CAPI providers. Should be set if not present in the Helm chart metadata. - properties: - bootstrap: - description: BootstrapProviders is the list of CAPI bootstrap - providers - items: - type: string - type: array - controlPlane: - description: ControlPlaneProviders is the list of CAPI control - plane providers - items: - type: string - type: array - infrastructure: - description: InfrastructureProviders is the list of CAPI infrastructure - providers - items: - type: string - type: array - type: object + items: + type: string + type: array required: - helm type: object @@ -188,26 +171,9 @@ spec: type: integer providers: description: Providers represent requested CAPI providers. - properties: - bootstrap: - description: BootstrapProviders is the list of CAPI bootstrap - providers - items: - type: string - type: array - controlPlane: - description: ControlPlaneProviders is the list of CAPI control - plane providers - items: - type: string - type: array - infrastructure: - description: InfrastructureProviders is the list of CAPI infrastructure - providers - items: - type: string - type: array - type: object + items: + type: string + type: array valid: description: Valid indicates whether the template passed validation or not. diff --git a/templates/provider/k0smotron/Chart.yaml b/templates/provider/k0smotron/Chart.yaml index 47ce14d17..9e1e376c2 100644 --- a/templates/provider/k0smotron/Chart.yaml +++ b/templates/provider/k0smotron/Chart.yaml @@ -20,6 +20,4 @@ version: 0.0.2 # It is recommended to use it with quotes. appVersion: "1.0.4" annotations: - hmc.mirantis.com/infrastructure-providers: k0smotron - hmc.mirantis.com/bootstrap-providers: k0s - hmc.mirantis.com/control-plane-providers: k0s; k0smotron + cluster.x-k8s.io/provider: infrastructure-k0smotron; bootstrap-k0s; control-plane-k0s; control-plane-k0smotron From 92e00c1de3ba9c6dc9d293d3d74832c8055e3ab3 Mon Sep 17 00:00:00 2001 From: zerospiel Date: Fri, 18 Oct 2024 17:24:42 +0200 Subject: [PATCH 2/3] Introduce compatibility based on contracts * removed version/constraints from compatibility attributes * compatibility is now based on CAPI contracts versions * unified providers structure * corresponding adjustments Closes #496 --- api/v1alpha1/clustertemplate_types.go | 16 ++-- api/v1alpha1/common.go | 31 +++---- api/v1alpha1/management_types.go | 4 +- api/v1alpha1/providertemplate_types.go | 86 ++++++------------- api/v1alpha1/servicetemplate_types.go | 4 +- api/v1alpha1/templates_common.go | 35 +++----- api/v1alpha1/zz_generated.deepcopy.go | 59 +++++-------- .../managedcluster_controller_test.go | 4 +- internal/controller/management_controller.go | 12 ++- internal/controller/template_controller.go | 55 +++++------- .../controller/template_controller_test.go | 32 +++---- .../webhook/managedcluster_webhook_test.go | 22 ++--- internal/webhook/management_webhook.go | 44 +++++----- internal/webhook/management_webhook_test.go | 68 ++++----------- .../hmc.mirantis.com_clustertemplates.yaml | 40 +++++---- .../crds/hmc.mirantis.com_managements.yaml | 20 +++-- .../hmc.mirantis.com_providertemplates.yaml | 71 +++++++-------- .../hmc.mirantis.com_servicetemplates.yaml | 34 +++++++- test/objects/management/management.go | 2 +- test/objects/template/template.go | 20 ++--- 20 files changed, 285 insertions(+), 374 deletions(-) diff --git a/api/v1alpha1/clustertemplate_types.go b/api/v1alpha1/clustertemplate_types.go index e8b211ba8..315d8c47e 100644 --- a/api/v1alpha1/clustertemplate_types.go +++ b/api/v1alpha1/clustertemplate_types.go @@ -33,19 +33,19 @@ type ClusterTemplateSpec struct { Helm HelmSpec `json:"helm"` // Kubernetes exact version in the SemVer format provided by this ClusterTemplate. KubernetesVersion string `json:"k8sVersion,omitempty"` - // Providers represent required CAPI providers with constrained compatibility versions set. + // Providers represent required CAPI providers with supported contract versions. // Should be set if not present in the Helm chart metadata. // Compatibility attributes are optional to be defined. - Providers ProvidersTupled `json:"providers,omitempty"` + Providers Providers `json:"providers,omitempty"` } // ClusterTemplateStatus defines the observed state of ClusterTemplate type ClusterTemplateStatus struct { // Kubernetes exact version in the SemVer format provided by this ClusterTemplate. KubernetesVersion string `json:"k8sVersion,omitempty"` - // Providers represent required CAPI providers with constrained compatibility versions set + // Providers represent required CAPI providers with supported contract versions // if the latter has been given. - Providers ProvidersTupled `json:"providers,omitempty"` + Providers Providers `json:"providers,omitempty"` TemplateStatusCommon `json:",inline"` } @@ -53,11 +53,7 @@ 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 { - var err error - t.Status.Providers, err = parseProviders(t, annotations, semver.NewConstraint) - if err != nil { - return fmt.Errorf("failed to parse ClusterTemplate providers: %v", err) - } + t.Status.Providers = parseProviders(t, annotations) kversion := annotations[ChartAnnotationKubernetesVersion] if t.Spec.KubernetesVersion != "" { @@ -77,7 +73,7 @@ func (t *ClusterTemplate) FillStatusWithProviders(annotations map[string]string) } // GetSpecProviders returns .spec.providers of the Template. -func (t *ClusterTemplate) GetSpecProviders() ProvidersTupled { +func (t *ClusterTemplate) GetSpecProviders() Providers { return t.Spec.Providers } diff --git a/api/v1alpha1/common.go b/api/v1alpha1/common.go index fdba2d2f6..3f6c8232a 100644 --- a/api/v1alpha1/common.go +++ b/api/v1alpha1/common.go @@ -36,29 +36,22 @@ const ( ) type ( - // TODO (zerospiel): unite with the versioned as part of the [Contracts support]. + // Holds different types of CAPI providers with [compatible contract versions]. // - // [Contracts support]: https://github.com/Mirantis/hmc/issues/496 + // [compatible contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts + Providers []NameContract - // Providers hold different types of CAPI providers. - Providers []string - - // Holds different types of CAPI providers with either - // an exact or constrained version in the SemVer format. The requirement - // is determined by a consumer of this type. - - // Holds different types of CAPI providers with either - // an exact or constrained version in the SemVer format. The requirement - // is determined by a consumer of this type. - ProvidersTupled []ProviderTuple - - // Represents name of the provider with either an exact or constrained version in the SemVer format. - ProviderTuple struct { + // Represents name of the provider with its provider compatibility [contract versions]. + // + // [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 SemVer format (exact or constrained version). + // Compatibility restriction in the [CAPI provider format]. The value is an underscore-delimited (_) list of versions. // Optional to be defined. - VersionOrConstraint string `json:"versionOrConstraint,omitempty"` + // + // [CAPI provider format]: https://cluster-api.sigs.k8s.io/developer/providers/contracts#api-version-labels + ContractVersion string `json:"contractVersion,omitempty"` } ) @@ -137,7 +130,7 @@ func ExtractServiceTemplateName(rawObj client.Object) []string { } // Names flattens the underlaying slice to provider names slice and returns it. -func (c ProvidersTupled) Names() []string { +func (c Providers) Names() []string { nn := make([]string, len(c)) for i, v := range c { nn[i] = v.Name diff --git a/api/v1alpha1/management_types.go b/api/v1alpha1/management_types.go index 7c036ab3b..bc1adf74b 100644 --- a/api/v1alpha1/management_types.go +++ b/api/v1alpha1/management_types.go @@ -92,8 +92,8 @@ type ManagementStatus struct { // Release indicates the current Release object. Release string `json:"release,omitempty"` // AvailableProviders holds all CAPI providers available along with - // their exact compatibility versions if specified in ProviderTemplates on the Management cluster. - AvailableProviders ProvidersTupled `json:"availableProviders,omitempty"` + // their supported contract versions, if specified in ProviderTemplates, on the Management cluster. + AvailableProviders Providers `json:"availableProviders,omitempty"` // ObservedGeneration is the last observed generation. ObservedGeneration int64 `json:"observedGeneration,omitempty"` } diff --git a/api/v1alpha1/providertemplate_types.go b/api/v1alpha1/providertemplate_types.go index ca7ebd6cf..15f682a8e 100644 --- a/api/v1alpha1/providertemplate_types.go +++ b/api/v1alpha1/providertemplate_types.go @@ -15,46 +15,39 @@ package v1alpha1 import ( - "fmt" - - "github.com/Masterminds/semver/v3" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( - // ChartAnnotationCAPIVersion is an annotation containing the CAPI exact version in the SemVer format associated with a ProviderTemplate. - ChartAnnotationCAPIVersion = "hmc.mirantis.com/capi-version" - // ChartAnnotationCAPIVersionConstraint is an annotation containing the CAPI version constraint in the SemVer format associated with a ProviderTemplate. - ChartAnnotationCAPIVersionConstraint = "hmc.mirantis.com/capi-version-constraint" + // ChartAnnotationCAPIContractVersion is an annotation containing the expected core CAPI contract version (e.g. v1beta1) associated with a ProviderTemplate. + ChartAnnotationCAPIContractVersion = "hmc.mirantis.com/capi-version" ) -// +kubebuilder:validation:XValidation:rule="!(has(self.capiVersion) && has(self.capiVersionConstraint))", message="Either capiVersion or capiVersionConstraint may be set, but not both" - // ProviderTemplateSpec defines the desired state of ProviderTemplate type ProviderTemplateSpec struct { Helm HelmSpec `json:"helm,omitempty"` - // CAPI exact version in the SemVer format. - // Applicable only for the cluster-api ProviderTemplate itself. - CAPIVersion string `json:"capiVersion,omitempty"` - // CAPI version constraint in the SemVer format indicating compatibility with the core CAPI. - // Not applicable for the cluster-api ProviderTemplate. - CAPIVersionConstraint string `json:"capiVersionConstraint,omitempty"` - // Providers represent exposed CAPI providers with exact compatibility versions set. + // 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"` + // Providers represent exposed CAPI providers with supported contract versions. // Should be set if not present in the Helm chart metadata. - // Compatibility attributes are optional to be defined. - Providers ProvidersTupled `json:"providers,omitempty"` + // Supported contract versions are optional to be defined. + Providers Providers `json:"providers,omitempty"` } // ProviderTemplateStatus defines the observed state of ProviderTemplate type ProviderTemplateStatus struct { - // CAPI exact version in the SemVer format. - // Applicable only for the capi Template itself. - CAPIVersion string `json:"capiVersion,omitempty"` - // CAPI version constraint in the SemVer format indicating compatibility with the core CAPI. - CAPIVersionConstraint string `json:"capiVersionConstraint,omitempty"` - // Providers represent exposed CAPI providers with exact compatibility versions set + // 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"` + // Providers represent exposed CAPI providers with supported contract versions // if the latter has been given. - Providers ProvidersTupled `json:"providers,omitempty"` + Providers Providers `json:"providers,omitempty"` TemplateStatusCommon `json:",inline"` } @@ -62,47 +55,24 @@ 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 { - var err error - t.Status.Providers, err = parseProviders(t, annotations, semver.NewVersion) - if err != nil { - return fmt.Errorf("failed to parse ProviderTemplate providers: %v", err) - } + t.Status.Providers = parseProviders(t, annotations) if t.Name == CoreCAPIName { - capiVersion := annotations[ChartAnnotationCAPIVersion] - if t.Spec.CAPIVersion != "" { - capiVersion = t.Spec.CAPIVersion - } - if capiVersion == "" { - return nil - } - - if _, err := semver.NewVersion(capiVersion); err != nil { - return fmt.Errorf("failed to parse CAPI version %s: %w", capiVersion, err) - } - - t.Status.CAPIVersion = capiVersion - } else { - capiConstraint := annotations[ChartAnnotationCAPIVersionConstraint] - if t.Spec.CAPIVersionConstraint != "" { - capiConstraint = t.Spec.CAPIVersionConstraint - } - if capiConstraint == "" { - return nil - } - - if _, err := semver.NewConstraint(capiConstraint); err != nil { - return fmt.Errorf("failed to parse CAPI version constraint %s: %w", capiConstraint, err) - } - - t.Status.CAPIVersionConstraint = capiConstraint + return nil } + requiredCAPIContract := annotations[ChartAnnotationCAPIContractVersion] + if t.Spec.CAPIContractVersion != "" { + requiredCAPIContract = t.Spec.CAPIContractVersion + } + + t.Status.CAPIContractVersion = requiredCAPIContract + return nil } // GetSpecProviders returns .spec.providers of the Template. -func (t *ProviderTemplate) GetSpecProviders() ProvidersTupled { +func (t *ProviderTemplate) GetSpecProviders() Providers { return t.Spec.Providers } diff --git a/api/v1alpha1/servicetemplate_types.go b/api/v1alpha1/servicetemplate_types.go index 70aaf781d..7bf0d5142 100644 --- a/api/v1alpha1/servicetemplate_types.go +++ b/api/v1alpha1/servicetemplate_types.go @@ -57,11 +57,11 @@ func (t *ServiceTemplate) FillStatusWithProviders(annotations map[string]string) t.Status.Providers = t.Spec.Providers } else { splitted := strings.Split(providers, multiProviderSeparator) - t.Status.Providers = make([]string, 0, len(splitted)) + 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, c) + t.Status.Providers = append(t.Status.Providers, NameContract{Name: c}) } } } diff --git a/api/v1alpha1/templates_common.go b/api/v1alpha1/templates_common.go index 688b28e86..5fe0ebfe8 100644 --- a/api/v1alpha1/templates_common.go +++ b/api/v1alpha1/templates_common.go @@ -15,8 +15,6 @@ package v1alpha1 import ( - "errors" - "fmt" "strings" helmcontrollerv2 "github.com/fluxcd/helm-controller/api/v2" @@ -74,49 +72,36 @@ type TemplateValidationStatus struct { Valid bool `json:"valid"` } -// TODO (zerospiel): change to comma as part of the [Contracts support]. -// -// [Contracts support]: https://github.com/Mirantis/hmc/issues/496 -const multiProviderSeparator = ";" +const multiProviderSeparator = "," -// TODO (zerospiel): move to the template-ctrl? -func parseProviders[T any](providersGetter interface{ GetSpecProviders() ProvidersTupled }, annotations map[string]string, validationFn func(string) (T, error)) ([]ProviderTuple, error) { +func parseProviders(providersGetter interface{ GetSpecProviders() Providers }, annotations map[string]string) []NameContract { providers := annotations[ChartAnnotationProviderName] if len(providers) == 0 { - return providersGetter.GetSpecProviders(), nil + return providersGetter.GetSpecProviders() } var ( ps = providersGetter.GetSpecProviders() splitted = strings.Split(providers, multiProviderSeparator) - pstatus = make([]ProviderTuple, 0, len(splitted)+len(ps)) - merr error + pstatus = make([]NameContract, 0, len(splitted)+len(ps)) ) pstatus = append(pstatus, ps...) for _, v := range splitted { v = strings.TrimSpace(v) - nVerOrC := strings.SplitN(v, " ", 2) - if len(nVerOrC) == 0 { // BCE (bound check elimination) + nameContract := strings.SplitN(v, " ", 2) + if len(nameContract) == 0 { // BCE (bound check elimination) continue } - n := ProviderTuple{Name: nVerOrC[0]} - if len(nVerOrC) < 2 { - pstatus = append(pstatus, n) - continue - } - - ver := strings.TrimSpace(nVerOrC[1]) - if _, err := validationFn(ver); err != nil { // validation - merr = errors.Join(merr, fmt.Errorf("failed to parse %s in the %s: %v", ver, v, err)) - continue + n := NameContract{Name: nameContract[0]} + if len(nameContract) > 1 { + n.ContractVersion = nameContract[1] } - n.VersionOrConstraint = ver pstatus = append(pstatus, n) } - return pstatus, merr + return pstatus } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 706a72b05..543a99e4d 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -190,7 +190,7 @@ func (in *ClusterTemplateSpec) DeepCopyInto(out *ClusterTemplateSpec) { in.Helm.DeepCopyInto(&out.Helm) if in.Providers != nil { in, out := &in.Providers, &out.Providers - *out = make(ProvidersTupled, len(*in)) + *out = make(Providers, len(*in)) copy(*out, *in) } } @@ -210,7 +210,7 @@ func (in *ClusterTemplateStatus) DeepCopyInto(out *ClusterTemplateStatus) { *out = *in if in.Providers != nil { in, out := &in.Providers, &out.Providers - *out = make(ProvidersTupled, len(*in)) + *out = make(Providers, len(*in)) copy(*out, *in) } in.TemplateStatusCommon.DeepCopyInto(&out.TemplateStatusCommon) @@ -613,7 +613,7 @@ func (in *ManagementStatus) DeepCopyInto(out *ManagementStatus) { } if in.AvailableProviders != nil { in, out := &in.AvailableProviders, &out.AvailableProviders - *out = make(ProvidersTupled, len(*in)) + *out = make(Providers, len(*in)) copy(*out, *in) } } @@ -725,6 +725,21 @@ 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 @@ -822,7 +837,7 @@ func (in *ProviderTemplateSpec) DeepCopyInto(out *ProviderTemplateSpec) { in.Helm.DeepCopyInto(&out.Helm) if in.Providers != nil { in, out := &in.Providers, &out.Providers - *out = make(ProvidersTupled, len(*in)) + *out = make(Providers, len(*in)) copy(*out, *in) } } @@ -842,7 +857,7 @@ func (in *ProviderTemplateStatus) DeepCopyInto(out *ProviderTemplateStatus) { *out = *in if in.Providers != nil { in, out := &in.Providers, &out.Providers - *out = make(ProvidersTupled, len(*in)) + *out = make(Providers, len(*in)) copy(*out, *in) } in.TemplateStatusCommon.DeepCopyInto(&out.TemplateStatusCommon) @@ -858,21 +873,6 @@ func (in *ProviderTemplateStatus) DeepCopy() *ProviderTemplateStatus { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ProviderTuple) DeepCopyInto(out *ProviderTuple) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProviderTuple. -func (in *ProviderTuple) DeepCopy() *ProviderTuple { - if in == nil { - return nil - } - out := new(ProviderTuple) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in Providers) DeepCopyInto(out *Providers) { { @@ -892,25 +892,6 @@ func (in Providers) DeepCopy() Providers { return *out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in ProvidersTupled) DeepCopyInto(out *ProvidersTupled) { - { - in := &in - *out = make(ProvidersTupled, len(*in)) - copy(*out, *in) - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProvidersTupled. -func (in ProvidersTupled) DeepCopy() ProvidersTupled { - if in == nil { - return nil - } - out := new(ProvidersTupled) - in.DeepCopyInto(out) - return *out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Release) DeepCopyInto(out *Release) { *out = *in diff --git a/internal/controller/managedcluster_controller_test.go b/internal/controller/managedcluster_controller_test.go index 029b544a9..9a8f9bfd0 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.ProvidersTupled{{Name: "infrastructure-aws"}}, + Providers: hmc.Providers{{Name: "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.ProvidersTupled{{Name: "infrastructure-aws"}}, + AvailableProviders: hmc.Providers{{Name: "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 537dc1e43..dc75f9d83 100644 --- a/internal/controller/management_controller.go +++ b/internal/controller/management_controller.go @@ -107,7 +107,7 @@ func (r *ManagementReconciler) Update(ctx context.Context, management *hmc.Manag } var errs error - detectedProviders := hmc.ProvidersTupled{} + detectedProviders := hmc.Providers{} detectedComponents := make(map[string]hmc.ComponentStatus) err := r.enableAdditionalComponents(ctx, management) @@ -414,10 +414,10 @@ func (r *ManagementReconciler) enableAdditionalComponents(ctx context.Context, m func updateComponentsStatus( components map[string]hmc.ComponentStatus, - providers *hmc.ProvidersTupled, + providers *hmc.Providers, componentName string, templateName string, - templateProviders hmc.ProvidersTupled, + templateProviders hmc.Providers, err string, ) { components[componentName] = hmc.ComponentStatus{ @@ -426,9 +426,13 @@ 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.ProviderTuple) int { return strings.Compare(a.Name, b.Name) }) + slices.SortFunc(*providers, func(a, b hmc.NameContract) int { return strings.Compare(a.Name, b.Name) }) *providers = slices.Compact(*providers) } } diff --git a/internal/controller/template_controller.go b/internal/controller/template_controller.go index 9e0c403a9..73f218354 100644 --- a/internal/controller/template_controller.go +++ b/internal/controller/template_controller.go @@ -20,9 +20,9 @@ import ( "errors" "fmt" "slices" + "strings" "time" - "github.com/Masterminds/semver/v3" helmcontrollerv2 "github.com/fluxcd/helm-controller/api/v2" sourcev1 "github.com/fluxcd/source-controller/api/v1" "helm.sh/helm/v3/pkg/chart" @@ -344,8 +344,8 @@ func (r *ClusterTemplateReconciler) validateCompatibilityAttrs(ctx context.Conte ctrl.LoggerFrom(ctx).V(1).Info("providers to check", "exposed", exposedProviders, "required", requiredProviders) var merr error - missing, wrong, parsing := collectMissingProvidersWithWrongVersions(exposedProviders, requiredProviders) - merr = errors.Join(merr, missing, wrong, parsing) + missing, wrong := collectMissingProvidersWithWrongVersions(exposedProviders, requiredProviders) + merr = errors.Join(merr, missing, wrong) if merr != nil { _ = r.updateStatus(ctx, template, merr.Error()) @@ -355,43 +355,36 @@ func (r *ClusterTemplateReconciler) validateCompatibilityAttrs(ctx context.Conte return r.updateStatus(ctx, template, "") } -// collectMissingProvidersWithWrongVersions returns collected errors for missing providers, providers with -// wrong versions that do not satisfy the corresponding constraints, and parsing errors respectevly. -func collectMissingProvidersWithWrongVersions(exposed, required []hmc.ProviderTuple) (missingErr, nonSatisfyingErr, parsingErr error) { - exposedSet := make(map[string]hmc.ProviderTuple, len(exposed)) +// 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 { - exposedSet[v.Name] = v + if v.ContractVersion != "" { + exposed2Contracts[v.Name] = strings.Split(v.ContractVersion, "_") + } else { + exposed2Contracts[v.Name] = nil + } } var missing, nonSatisfying []string - for _, reqWithConstraint := range required { - exposedWithExactVer, ok := exposedSet[reqWithConstraint.Name] + for _, requiredProvider := range required { + exposedContracts, ok := exposed2Contracts[requiredProvider.Name] if !ok { - missing = append(missing, reqWithConstraint.Name) - continue - } - - version := exposedWithExactVer.VersionOrConstraint - constraint := reqWithConstraint.VersionOrConstraint - - if version == "" || constraint == "" { + missing = append(missing, requiredProvider.Name) continue } - exactVer, err := semver.NewVersion(version) - if err != nil { - parsingErr = errors.Join(parsingErr, fmt.Errorf("failed to parse version %s of the provider %s: %w", version, exposedWithExactVer.Name, err)) + if len(exposedContracts) == 0 || requiredProvider.ContractVersion == "" { continue } - requiredC, err := semver.NewConstraint(constraint) - if err != nil { - parsingErr = errors.Join(parsingErr, fmt.Errorf("failed to parse constraint %s of the provider %s: %w", version, exposedWithExactVer.Name, err)) - continue - } + requiredContracts := strings.Split(requiredProvider.ContractVersion, "_") // must be only one, but better to check twice - if !requiredC.Check(exactVer) { - nonSatisfying = append(nonSatisfying, fmt.Sprintf("%s %s !~ %s", reqWithConstraint.Name, version, constraint)) + for _, rc := range requiredContracts { + if !slices.Contains(exposedContracts, rc) { + nonSatisfying = append(nonSatisfying, fmt.Sprintf("%s %s !~ %v", requiredProvider.Name, rc, exposedContracts)) + } } } @@ -405,11 +398,7 @@ func collectMissingProvidersWithWrongVersions(exposed, required []hmc.ProviderTu nonSatisfyingErr = fmt.Errorf("one or more required providers does not satisfy constraints: %v", nonSatisfying) } - if parsingErr != nil { - parsingErr = fmt.Errorf("one or more errors parsing providers' versions and constraints : %v", parsingErr) - } - - return missingErr, nonSatisfyingErr, parsingErr + return missingErr, nonSatisfyingErr } // 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 fe6cf6d4a..5d011426f 100644 --- a/internal/controller/template_controller_test.go +++ b/internal/controller/template_controller_test.go @@ -198,11 +198,11 @@ var _ = Describe("Template Controller", func() { It("should successfully validate cluster templates providers compatibility attributes", func() { const ( - clusterTemplateName = "cluster-template-test-name" - mgmtName = hmcmirantiscomv1alpha1.ManagementName - someProviderName = "test-provider-name" - someProviderVersion = "v1.0.0" - someProviderVersionConstraint = ">= 1.0.0 <2.0.0-0" // ^1.0.0 + clusterTemplateName = "cluster-template-test-name" + mgmtName = hmcmirantiscomv1alpha1.ManagementName + someProviderName = "test-provider-name" + someRequiredContract = "v1beta1" + someExposedContract = "v1beta1_v1beta2" timeout = time.Second * 10 interval = time.Millisecond * 250 @@ -217,10 +217,10 @@ var _ = Describe("Template Controller", func() { }, Spec: hmcmirantiscomv1alpha1.ClusterTemplateSpec{ Helm: helmSpec, - Providers: []hmcmirantiscomv1alpha1.ProviderTuple{ + Providers: []hmcmirantiscomv1alpha1.NameContract{ { - Name: someProviderName, - VersionOrConstraint: someProviderVersionConstraint, // constraint + Name: someProviderName, + ContractVersion: someRequiredContract, }, }, }, @@ -237,8 +237,8 @@ var _ = Describe("Template Controller", func() { return fmt.Errorf("expected .spec.providers length to be exactly 1, got %d", l) } - if v := clusterTemplate.Spec.Providers[0]; v.Name != someProviderName || v.VersionOrConstraint != someProviderVersionConstraint { - return fmt.Errorf("expected .spec.providers[0] to be %s:%s, got %s:%s", someProviderName, someProviderVersionConstraint, v.Name, v.VersionOrConstraint) + 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) } return nil @@ -253,10 +253,10 @@ var _ = Describe("Template Controller", func() { } Expect(k8sClient.Create(ctx, mgmt)).To(Succeed()) mgmt.Status = hmcmirantiscomv1alpha1.ManagementStatus{ - AvailableProviders: []hmcmirantiscomv1alpha1.ProviderTuple{ + AvailableProviders: []hmcmirantiscomv1alpha1.NameContract{ { - Name: someProviderName, - VersionOrConstraint: someProviderVersion, // version + Name: someProviderName, + ContractVersion: someExposedContract, }, }, } @@ -272,8 +272,8 @@ var _ = Describe("Template Controller", func() { return fmt.Errorf("expected .status.availableProviders length to be exactly 1, got %d", l) } - if v := mgmt.Status.AvailableProviders[0]; v.Name != someProviderName || v.VersionOrConstraint != someProviderVersion { - return fmt.Errorf("expected .status.availableProviders[0] to be %s:%s, got %s:%s", someProviderName, someProviderVersionConstraint, v.Name, v.VersionOrConstraint) + 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) } return nil @@ -294,7 +294,7 @@ 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.ProviderTuple{Name: someProviderName, VersionOrConstraint: someProviderVersionConstraint})) + Expect(clusterTemplate.Status.Providers[0]).To(Equal(hmcmirantiscomv1alpha1.NameContract{Name: someProviderName, ContractVersion: someRequiredContract})) By("Removing the created objects") Expect(k8sClient.Delete(ctx, mgmt)).To(Succeed()) diff --git a/internal/webhook/managedcluster_webhook_test.go b/internal/webhook/managedcluster_webhook_test.go index ad8261de6..398f3a507 100644 --- a/internal/webhook/managedcluster_webhook_test.go +++ b/internal/webhook/managedcluster_webhook_test.go @@ -42,7 +42,7 @@ var ( testNamespace = "test" mgmt = management.NewManagement( - management.WithAvailableProviders(v1alpha1.ProvidersTupled{ + management.WithAvailableProviders(v1alpha1.Providers{ {Name: "infrastructure-aws"}, {Name: "control-plane-k0s"}, {Name: "bootstrap-k0s"}, @@ -127,7 +127,7 @@ func TestManagedClusterValidateCreate(t *testing.T) { cred, template.NewClusterTemplate( template.WithName(testTemplateName), - template.WithProvidersStatus(v1alpha1.ProvidersTupled{ + template.WithProvidersStatus(v1alpha1.Providers{ {Name: "infrastructure-aws"}, {Name: "control-plane-k0s"}, {Name: "bootstrap-k0s"}, @@ -144,10 +144,10 @@ func TestManagedClusterValidateCreate(t *testing.T) { ), existingObjects: []runtime.Object{ cred, - management.NewManagement(management.WithAvailableProviders(v1alpha1.ProvidersTupled{ - {Name: "infrastructure-aws", VersionOrConstraint: "v1.0.0"}, - {Name: "control-plane-k0s", VersionOrConstraint: "v1.0.0"}, - {Name: "bootstrap-k0s", VersionOrConstraint: "v1.0.0"}, + management.NewManagement(management.WithAvailableProviders(v1alpha1.Providers{ + {Name: "infrastructure-aws"}, + {Name: "control-plane-k0s"}, + {Name: "bootstrap-k0s"}, })), template.NewClusterTemplate( template.WithName(testTemplateName), @@ -170,7 +170,7 @@ func TestManagedClusterValidateCreate(t *testing.T) { mgmt, template.NewClusterTemplate( template.WithName(testTemplateName), - template.WithProvidersStatus(v1alpha1.ProvidersTupled{ + template.WithProvidersStatus(v1alpha1.Providers{ {Name: "infrastructure-aws"}, {Name: "control-plane-k0s"}, {Name: "bootstrap-k0s"}, @@ -199,7 +199,7 @@ func TestManagedClusterValidateCreate(t *testing.T) { ), template.NewClusterTemplate( template.WithName(testTemplateName), - template.WithProvidersStatus(v1alpha1.ProvidersTupled{ + template.WithProvidersStatus(v1alpha1.Providers{ {Name: "infrastructure-aws"}, {Name: "control-plane-k0s"}, {Name: "bootstrap-k0s"}, @@ -218,7 +218,7 @@ func TestManagedClusterValidateCreate(t *testing.T) { existingObjects: []runtime.Object{ cred, management.NewManagement( - management.WithAvailableProviders(v1alpha1.ProvidersTupled{ + management.WithAvailableProviders(v1alpha1.Providers{ {Name: "infrastructure-azure"}, {Name: "control-plane-k0s"}, {Name: "bootstrap-k0s"}, @@ -226,7 +226,7 @@ func TestManagedClusterValidateCreate(t *testing.T) { ), template.NewClusterTemplate( template.WithName(testTemplateName), - template.WithProvidersStatus(v1alpha1.ProvidersTupled{ + template.WithProvidersStatus(v1alpha1.Providers{ {Name: "infrastructure-azure"}, {Name: "control-plane-k0s"}, {Name: "bootstrap-k0s"}, @@ -310,7 +310,7 @@ func TestManagedClusterValidateUpdate(t *testing.T) { Valid: false, ValidationError: "validation error example", }), - template.WithProvidersStatus(v1alpha1.ProvidersTupled{ + template.WithProvidersStatus(v1alpha1.Providers{ {Name: "infrastructure-aws"}, {Name: "control-plane-k0s"}, {Name: "bootstrap-k0s"}, diff --git a/internal/webhook/management_webhook.go b/internal/webhook/management_webhook.go index b0c0f1704..c15f38cfd 100644 --- a/internal/webhook/management_webhook.go +++ b/internal/webhook/management_webhook.go @@ -18,6 +18,8 @@ import ( "context" "errors" "fmt" + "slices" + "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -26,7 +28,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "github.com/Masterminds/semver/v3" hmcv1alpha1 "github.com/Mirantis/hmc/api/v1alpha1" ) @@ -78,19 +79,18 @@ func (v *ManagementValidator) ValidateUpdate(ctx context.Context, _, newObj runt capiTplName = mgmt.Spec.Core.CAPI.Template } - 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 capiTpl.Status.CAPIVersion == "" { - return nil, nil // nothing to validate against - } + 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) + // } - capiRequiredVersion, err := semver.NewVersion(capiTpl.Status.CAPIVersion) - if err != nil { // should never happen - return nil, fmt.Errorf("%s: invalid CAPI version %s in the ProviderTemplate %s to be validated against: %v", invalidMgmtMsg, capiTpl.Status.CAPIVersion, capiTpl.Name, err) - } + // 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) + // } var wrongVersions error for _, p := range mgmt.Spec.Providers { @@ -99,7 +99,7 @@ func (v *ManagementValidator) ValidateUpdate(ctx context.Context, _, newObj runt tplName = release.ProviderTemplate(p.Name) } - if tplName == capiTpl.Name { // skip capi itself + if tplName == capiTplName { // skip capi itself continue } @@ -108,22 +108,20 @@ func (v *ManagementValidator) ValidateUpdate(ctx context.Context, _, newObj runt return nil, fmt.Errorf("failed to get ProviderTemplate %s: %w", tplName, err) } - if pTpl.Status.CAPIVersionConstraint == "" { + if pTpl.Status.CAPIContractVersion == "" { continue } - constraint, err := semver.NewConstraint(pTpl.Status.CAPIVersionConstraint) - if err != nil { // should never happen - return nil, fmt.Errorf("%s: invalid CAPI version constraint %s in the ProviderTemplate %s: %v", invalidMgmtMsg, pTpl.Status.CAPIVersionConstraint, pTpl.Name, err) - } - - if !constraint.Check(capiRequiredVersion) { - wrongVersions = errors.Join(wrongVersions, fmt.Errorf("core CAPI version %s does not satisfy ProviderTemplate %s constraint %s", capiRequiredVersion, pTpl.Name, constraint)) + 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 wrongVersions != nil { - return admission.Warnings{"The Management object has incompatible CAPI versions ProviderTemplates"}, fmt.Errorf("%s: %s", invalidMgmtMsg, wrongVersions) + return admission.Warnings{"The Management object has incompatible CAPI contract versions in ProviderTemplates"}, fmt.Errorf("%s: %s", invalidMgmtMsg, wrongVersions) } return nil, nil diff --git a/internal/webhook/management_webhook_test.go b/internal/webhook/management_webhook_test.go index a3d311475..e56eccf5a 100644 --- a/internal/webhook/management_webhook_test.go +++ b/internal/webhook/management_webhook_test.go @@ -39,10 +39,8 @@ func TestManagementValidateUpdate(t *testing.T) { ctx := admission.NewContextWithRequest(context.Background(), admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Update}}) const ( - versionOne = "1.0.0" - constraintVerOne, constraintVerTwo = "^1.0.0", "~2.0.0" - invalidVersion = "invalid-ver" - invalidConstraint = "invalid-constraint" + contractVersionValid = "v1alpha4_v1beta1" + contractVersionInvalid = "v1alpha1_v1alpha4_v1beta1" ) providerAwsDefaultTpl := v1alpha1.Provider{ @@ -52,6 +50,8 @@ func TestManagementValidateUpdate(t *testing.T) { }, } + supportedCAPIContractVersions := []string{"v1alpha3", "v1alpha4", "v1beta1"} + tests := []struct { name string management *v1alpha1.Management @@ -64,48 +64,16 @@ func TestManagementValidateUpdate(t *testing.T) { management: management.NewManagement(), }, { - 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 with wrong capi semver set, should fail", - management: management.NewManagement(management.WithRelease(release.DefaultName)), - existingObjects: []runtime.Object{ - release.New(), - template.NewProviderTemplate( - template.WithName(release.DefaultCAPITemplateName), - template.WithProviderStatusCAPIVersion(invalidVersion), - ), - }, - err: fmt.Sprintf("the Management is invalid: invalid CAPI version %s in the ProviderTemplate %s to be validated against: Invalid Semantic Version", invalidVersion, release.DefaultCAPITemplateName), - }, - { - name: "providertemplates without specified capi constraints, should succeed", + name: "no providertemplates having providers in mgmt spec, should fail", management: management.NewManagement( management.WithRelease(release.DefaultName), management.WithProviders([]v1alpha1.Provider{providerAwsDefaultTpl}), ), - existingObjects: []runtime.Object{ - release.New(), - template.NewProviderTemplate( - template.WithName(release.DefaultCAPITemplateName), - template.WithProviderStatusCAPIVersion(versionOne), - ), - template.NewProviderTemplate(), - }, + existingObjects: []runtime.Object{release.New()}, + err: fmt.Sprintf(`failed to get ProviderTemplate %s: providertemplates.hmc.mirantis.com "%s" not found`, template.DefaultName, template.DefaultName), }, { - name: "providertemplates with invalid specified capi semver, should fail", + name: "providertemplates without specified capi contracts, should succeed", management: management.NewManagement( management.WithRelease(release.DefaultName), management.WithProviders([]v1alpha1.Provider{providerAwsDefaultTpl}), @@ -114,16 +82,12 @@ func TestManagementValidateUpdate(t *testing.T) { release.New(), template.NewProviderTemplate( template.WithName(release.DefaultCAPITemplateName), - template.WithProviderStatusCAPIVersion(versionOne), - ), - template.NewProviderTemplate( - template.WithProviderStatusCAPIConstraint(invalidConstraint), ), + template.NewProviderTemplate(), }, - err: fmt.Sprintf("the Management is invalid: invalid CAPI version constraint %s in the ProviderTemplate %s: improper constraint: %s", invalidConstraint, template.DefaultName, invalidConstraint), }, { - name: "providertemplates do not match capi version, should fail", + name: "providertemplates do not match capi contracts, should fail", management: management.NewManagement( management.WithRelease(release.DefaultName), management.WithProviders([]v1alpha1.Provider{providerAwsDefaultTpl}), @@ -132,17 +96,16 @@ func TestManagementValidateUpdate(t *testing.T) { release.New(), template.NewProviderTemplate( template.WithName(release.DefaultCAPITemplateName), - template.WithProviderStatusCAPIVersion(versionOne), ), template.NewProviderTemplate( - template.WithProviderStatusCAPIConstraint(constraintVerTwo), + template.WithProviderStatusCAPIContract(contractVersionInvalid), ), }, - warnings: admission.Warnings{"The Management object has incompatible CAPI versions ProviderTemplates"}, - err: fmt.Sprintf("the Management is invalid: core CAPI version %s does not satisfy ProviderTemplate %s constraint %s", versionOne, template.DefaultName, constraintVerTwo), + 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"), }, { - name: "providertemplates match capi version, should succeed", + name: "providertemplates match capi contracts, should succeed", management: management.NewManagement( management.WithRelease(release.DefaultName), management.WithProviders([]v1alpha1.Provider{providerAwsDefaultTpl}), @@ -151,10 +114,9 @@ func TestManagementValidateUpdate(t *testing.T) { release.New(), template.NewProviderTemplate( template.WithName(release.DefaultCAPITemplateName), - template.WithProviderStatusCAPIVersion(versionOne), ), template.NewProviderTemplate( - template.WithProviderStatusCAPIConstraint(constraintVerOne), + template.WithProviderStatusCAPIContract(contractVersionValid), ), }, }, 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 bd386db7d..fafcd2e0e 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_clustertemplates.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_clustertemplates.yaml @@ -109,20 +109,24 @@ spec: type: string providers: description: |- - Providers represent required CAPI providers with constrained compatibility versions set. + Providers represent required CAPI providers with supported contract versions. 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 either an exact - or constrained version in the SemVer format. + description: |- + Represents name of the provider with its provider compatibility [contract versions]. + + [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: + contractVersion: description: |- - Compatibility restriction in the SemVer format (exact or constrained version). + 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 @@ -183,19 +187,23 @@ spec: type: integer providers: description: |- - Providers represent required CAPI providers with constrained compatibility versions set + Providers represent required CAPI providers with supported contract versions if the latter has been given. items: - description: Represents name of the provider with either an exact - or constrained version in the SemVer format. + description: |- + Represents name of the provider with its provider compatibility [contract versions]. + + [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: + contractVersion: description: |- - Compatibility restriction in the SemVer format (exact or constrained version). + 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 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 d69574975..f8df43e48 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_managements.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_managements.yaml @@ -114,18 +114,22 @@ spec: availableProviders: description: |- AvailableProviders holds all CAPI providers available along with - their exact compatibility versions if specified in ProviderTemplates on the Management cluster. + their supported contract versions, if specified in ProviderTemplates, on the Management cluster. items: - description: Represents name of the provider with either an exact - or constrained version in the SemVer format. + description: |- + Represents name of the provider with its provider compatibility [contract versions]. + + [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: + contractVersion: description: |- - Compatibility restriction in the SemVer format (exact or constrained version). + 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 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 1e584155f..a50fbeb2d 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_providertemplates.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_providertemplates.yaml @@ -56,15 +56,13 @@ spec: spec: description: ProviderTemplateSpec defines the desired state of ProviderTemplate properties: - capiVersion: + capiContractVersion: description: |- - CAPI exact version in the SemVer format. - Applicable only for the cluster-api ProviderTemplate itself. - type: string - capiVersionConstraint: - description: |- - CAPI version constraint in the SemVer format indicating compatibility with the core CAPI. - Not applicable for the cluster-api ProviderTemplate. + 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 type: string helm: description: HelmSpec references a Helm chart representing the HMC @@ -115,20 +113,24 @@ spec: && has(self.chartRef)) providers: description: |- - Providers represent exposed CAPI providers with exact compatibility versions set. + Providers represent exposed CAPI providers with supported contract versions. Should be set if not present in the Helm chart metadata. - Compatibility attributes are optional to be defined. + Supported contract versions are optional to be defined. items: - description: Represents name of the provider with either an exact - or constrained version in the SemVer format. + description: |- + Represents name of the provider with its provider compatibility [contract versions]. + + [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: + contractVersion: description: |- - Compatibility restriction in the SemVer format (exact or constrained version). + 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 @@ -136,20 +138,15 @@ spec: x-kubernetes-validations: - message: Spec is immutable rule: self == oldSelf - - message: Either capiVersion or capiVersionConstraint may be set, but - not both - rule: '!(has(self.capiVersion) && has(self.capiVersionConstraint))' status: description: ProviderTemplateStatus defines the observed state of ProviderTemplate properties: - capiVersion: + capiContractVersion: description: |- - CAPI exact version in the SemVer format. - Applicable only for the capi Template itself. - type: string - capiVersionConstraint: - description: CAPI version constraint in the SemVer format indicating - compatibility with the core CAPI. + 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 type: string chartRef: description: |- @@ -195,19 +192,23 @@ spec: type: integer providers: description: |- - Providers represent exposed CAPI providers with exact compatibility versions set + Providers represent exposed CAPI providers with supported contract versions if the latter has been given. items: - description: Represents name of the provider with either an exact - or constrained version in the SemVer format. + description: |- + Represents name of the provider with its provider compatibility [contract versions]. + + [contract versions]: https://cluster-api.sigs.k8s.io/developer/providers/contracts properties: - name: - description: Name of the provider. - type: string - versionOrConstraint: + contractVersion: description: |- - Compatibility restriction in the SemVer format (exact or constrained version). + 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 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 84c17f57f..48800b1fe 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_servicetemplates.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_servicetemplates.yaml @@ -112,7 +112,22 @@ spec: Providers represent requested CAPI providers. Should be set if not present in the Helm chart metadata. items: - type: string + 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: array required: - helm @@ -172,7 +187,22 @@ spec: providers: description: Providers represent requested CAPI providers. items: - type: string + 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: array valid: description: Valid indicates whether the template passed validation diff --git a/test/objects/management/management.go b/test/objects/management/management.go index a6861c804..b120c6be5 100644 --- a/test/objects/management/management.go +++ b/test/objects/management/management.go @@ -64,7 +64,7 @@ func WithProviders(providers []v1alpha1.Provider) Opt { } } -func WithAvailableProviders(providers v1alpha1.ProvidersTupled) Opt { +func WithAvailableProviders(providers v1alpha1.Providers) Opt { return func(p *v1alpha1.Management) { p.Status.AvailableProviders = providers } diff --git a/test/objects/template/template.go b/test/objects/template/template.go index f53877503..9ac4cf412 100644 --- a/test/objects/template/template.go +++ b/test/objects/template/template.go @@ -140,18 +140,18 @@ func WithValidationStatus(validationStatus v1alpha1.TemplateValidationStatus) Op } } -func WithProvidersStatus[T v1alpha1.Providers | v1alpha1.ProvidersTupled](providers T) Opt { +func WithProvidersStatus(providers v1alpha1.Providers) Opt { return func(t Template) { switch v := t.(type) { case *v1alpha1.ClusterTemplate: var ok bool - v.Status.Providers, ok = any(providers).(v1alpha1.ProvidersTupled) + v.Status.Providers, ok = any(providers).(v1alpha1.Providers) if !ok { panic(fmt.Sprintf("unexpected type %T", providers)) } case *v1alpha1.ProviderTemplate: var ok bool - v.Status.Providers, ok = any(providers).(v1alpha1.ProvidersTupled) + v.Status.Providers, ok = any(providers).(v1alpha1.Providers) if !ok { panic(fmt.Sprintf("unexpected type %T", providers)) } @@ -174,23 +174,13 @@ func WithConfigStatus(config string) Opt { } } -func WithProviderStatusCAPIVersion(v string) Opt { - return func(template Template) { - pt, ok := template.(*v1alpha1.ProviderTemplate) - if !ok { - panic(fmt.Sprintf("unexpected type %T, expected ProviderTemplate", template)) - } - pt.Status.CAPIVersion = v - } -} - -func WithProviderStatusCAPIConstraint(v string) Opt { +func WithProviderStatusCAPIContract(v string) Opt { return func(template Template) { pt, ok := template.(*v1alpha1.ProviderTemplate) if !ok { panic(fmt.Sprintf("unexpected type %T, expected ProviderTemplate", template)) } - pt.Status.CAPIVersionConstraint = v + pt.Status.CAPIContractVersion = v } } From ad9af2851d91211d18fabba22129233ec99c3938 Mon Sep 17 00:00:00 2001 From: zerospiel Date: Tue, 22 Oct 2024 14:02:41 +0200 Subject: [PATCH 3/3] Compatibility rework: addressed comments * change semicolon to comma as a separator in the provider templates charts * capi-contract annotation removed * unified separated capi contracts struct for cluster/provider templates * capi contracts status in management status * provider templates: validate capi contracts against core capi template * cluster templates: validate provider contracts against provider templates contracts in management status * renamed k0s -> k0smotron everywhere --- api/v1alpha1/clustertemplate_types.go | 20 ++++- api/v1alpha1/common.go | 30 ++----- api/v1alpha1/compatibility_contract.go | 74 +++++++++++++++++ api/v1alpha1/compatibility_contract_test.go | 72 ++++++++++++++++ api/v1alpha1/management_types.go | 7 ++ api/v1alpha1/providertemplate_types.go | 41 ++++------ api/v1alpha1/servicetemplate_types.go | 22 ++--- api/v1alpha1/templates_common.go | 80 ++++++++++++++---- api/v1alpha1/zz_generated.deepcopy.go | 82 +++++++++++++++---- .../controller/managedcluster_controller.go | 10 ++- .../managedcluster_controller_test.go | 4 +- internal/controller/management_controller.go | 32 +++++--- internal/controller/template_controller.go | 74 ++++++++--------- .../controller/template_controller_test.go | 45 +++++----- internal/telemetry/tracker.go | 2 +- internal/webhook/managedcluster_webhook.go | 14 ++-- .../webhook/managedcluster_webhook_test.go | 48 +++++------ internal/webhook/management_webhook.go | 39 ++++----- internal/webhook/management_webhook_test.go | 81 +++++++++++++++--- templates/cluster/aws-hosted-cp/Chart.yaml | 2 +- .../cluster/aws-standalone-cp/Chart.yaml | 2 +- templates/cluster/azure-hosted-cp/Chart.yaml | 2 +- .../cluster/azure-standalone-cp/Chart.yaml | 2 +- .../cluster/vsphere-hosted-cp/Chart.yaml | 2 +- .../cluster/vsphere-standalone-cp/Chart.yaml | 2 +- .../hmc.mirantis.com_clustertemplates.yaml | 56 ++++++------- .../crds/hmc.mirantis.com_managements.yaml | 31 ++++--- .../hmc.mirantis.com_providertemplates.yaml | 63 +++++--------- .../hmc.mirantis.com_servicetemplates.yaml | 34 +------- templates/provider/k0smotron/Chart.yaml | 2 +- test/objects/template/template.go | 19 ++++- 31 files changed, 628 insertions(+), 366 deletions(-) create mode 100644 api/v1alpha1/compatibility_contract.go create mode 100644 api/v1alpha1/compatibility_contract_test.go 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..90b801606 100644 --- a/api/v1alpha1/management_types.go +++ b/api/v1alpha1/management_types.go @@ -87,6 +87,13 @@ func GetDefaultProviders() []Provider { // ManagementStatus defines the observed state of Management type ManagementStatus struct { + // For each CAPI provider name holds its compatibility [contract versions] + // in a key-value pairs, 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 + 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..98c840af6 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,77 @@ 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) + } + } + + slices.Sort(pstatus) + return slices.Compact(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..97f462c71 100644 --- a/internal/controller/managedcluster_controller.go +++ b/internal/controller/managedcluster_controller.go @@ -585,10 +585,14 @@ func (r *ManagedClusterReconciler) getInfraProvidersNames(ctx context.Context, t return nil, err } - ips := make([]string, 0, len(template.Status.Providers)) + const infraPrefix = "infrastructure-" + var ( + ips = make([]string, 0, len(template.Status.Providers)) + lprefix = len(infraPrefix) + ) for _, v := range template.Status.Providers { - if strings.HasPrefix(v.Name, "infrastructure-") { - ips = append(ips, v.Name) + if idx := strings.Index(v, infraPrefix); idx > -1 { + ips = append(ips, v[idx+lprefix:]) } } 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..af53adbdc 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,28 @@ 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 + description: |- + For each CAPI provider name holds its compatibility [contract versions] + in a key-value pairs, 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 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] + } } }