Skip to content

Commit

Permalink
Addressed comments on compatibility attrs
Browse files Browse the repository at this point in the history
* amends to descs and fix typos
* correctly parse providers
* changed providers anno separator
* enforce CAPI version check
  in providertemplates
  • Loading branch information
zerospiel committed Oct 11, 2024
1 parent d438bcc commit aad600e
Show file tree
Hide file tree
Showing 24 changed files with 435 additions and 211 deletions.
11 changes: 7 additions & 4 deletions api/v1alpha1/clustertemplate_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,20 @@ const (
// ClusterTemplateSpec defines the desired state of ClusterTemplate
type ClusterTemplateSpec struct {
Helm HelmSpec `json:"helm"`
// Compatible K8S version of the cluster set in the SemVer format.
// Kubernetes exact version in the SemVer format required for this ClusterTemplate.
KubertenesVersion string `json:"k8sVersion,omitempty"`
// Providers represent required CAPI providers with constrainted compatibility versions set. Should be set if not present in the Helm chart metadata.
// 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.
Providers ProvidersTupled `json:"providers,omitempty"`
}

// ClusterTemplateStatus defines the observed state of ClusterTemplate
type ClusterTemplateStatus struct {
// Compatible K8S version of the cluster set in the SemVer format.
// Kubernetes exact version in the SemVer format required for this ClusterTemplate.
KubertenesVersion string `json:"k8sVersion,omitempty"`
// Providers represent exposed CAPI providers with constrainted compatibility versions set.
// Providers represent exposed CAPI providers with constrained compatibility versions set
// if the latter has been given.
Providers ProvidersTupled `json:"providers,omitempty"`

TemplateStatusCommon `json:",inline"`
Expand Down
16 changes: 10 additions & 6 deletions api/v1alpha1/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,26 @@ type (
}

// Holds different types of CAPI providers with either
// an exact or constrainted version in the SemVer format. The requirement
// 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 constrainted version in the SemVer format.
// 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 constrainted version in the SemVer format.
// 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 constrainted version in the SemVer format.
// 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"`
}

// Represents name of the provider with either an exact or constrainted version in the SemVer format.
// Represents name of the provider with either an exact or constrained version in the SemVer format.
ProviderTuple struct {
// Name of the provider.
Name string `json:"name,omitempty"`
// Compatibility restriction in the SemVer format (exact or constrainted version)
// Compatibility restriction in the SemVer format (exact or constrained version).
// Optional to be defined.
VersionOrConstraint string `json:"versionOrConstraint,omitempty"`
}
)
Expand Down
5 changes: 1 addition & 4 deletions api/v1alpha1/managedcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,9 @@ type ManagedClusterSpec struct {

// ManagedClusterStatus defines the observed state of ManagedCluster
type ManagedClusterStatus struct {
// Currently compatible K8S version of the cluster. Being set only if
// Currently compatible exact Kubernetes version of the cluster. Being set only if
// provided by the corresponding ClusterTemplate.
KubertenesVersion string `json:"k8sVersion,omitempty"`
// Providers represent exposed CAPI providers with constrainted compatibility versions set.
// Propagated from the corresponding ClusterTemplate.
Providers ProvidersTupled `json:"providers,omitempty"`
// Conditions contains details for the current state of the ManagedCluster
Conditions []metav1.Condition `json:"conditions,omitempty"`
// ObservedGeneration is the last observed generation.
Expand Down
15 changes: 11 additions & 4 deletions api/v1alpha1/providertemplate_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,24 @@ const ChartAnnotationCAPIVersion = "hmc.mirantis.com/capi-version"
// ProviderTemplateSpec defines the desired state of ProviderTemplate
type ProviderTemplateSpec struct {
Helm HelmSpec `json:"helm"`
// Compatible CAPI provider version set in the SemVer format.
// CAPI exact version in the SemVer format required for this ProviderTemplate.
// For the CAPI component it is the actual current version of the component
// against which other Templates will be validated.
CAPIVersion string `json:"capiVersion,omitempty"`
// Represents required CAPI providers with exact compatibility versions set. Should be set if not present in the Helm chart metadata.
// Providers represent required 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.
Providers ProvidersTupled `json:"providers,omitempty"`
}

// ProviderTemplateStatus defines the observed state of ProviderTemplate
type ProviderTemplateStatus struct {
// Compatible CAPI provider version in the SemVer format.
// CAPI exact version in the SemVer format required for this ProviderTemplate.
// For the CAPI component it is the actual current version of the component
// against which other Templates will be validated.
CAPIVersion string `json:"capiVersion,omitempty"`
// Providers represent exposed CAPI providers with exact compatibility versions set.
// Providers represent exposed CAPI providers with exact compatibility versions set
// if the latter has been given.
Providers ProvidersTupled `json:"providers,omitempty"`

TemplateStatusCommon `json:",inline"`
Expand Down
16 changes: 7 additions & 9 deletions api/v1alpha1/servicetemplate_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
const (
// Denotes the servicetemplate resource Kind.
ServiceTemplateKind = "ServiceTemplate"
// ChartAnnotationKubernetesConstraint is an annotation containing the Kubernetes constrainted version in the SemVer format associated with a ServiceTemplate.
// ChartAnnotationKubernetesConstraint is an annotation containing the Kubernetes constrained version in the SemVer format associated with a ServiceTemplate.
ChartAnnotationKubernetesConstraint = "hmc.mirantis.com/k8s-version-constraint"
)

Expand All @@ -34,15 +34,16 @@ type ServiceTemplateSpec struct {
Helm HelmSpec `json:"helm"`
// Constraint describing compatible K8S versions of the cluster set in the SemVer format.
KubertenesConstraint string `json:"k8sConstraint,omitempty"`
// Represents required CAPI providers. Should be set if not present in the Helm chart metadata.
// Providers represent required CAPI providers.
// Should be set if not present in the Helm chart metadata.
Providers Providers `json:"providers,omitempty"`
}

// ServiceTemplateStatus defines the observed state of ServiceTemplate
type ServiceTemplateStatus struct {
// Constraint describing compatible K8S versions of the cluster set in the SemVer format.
KubertenesConstraint string `json:"k8sConstraint,omitempty"`
// Represents exposed CAPI providers.
// Providers represent requested CAPI providers.
Providers Providers `json:"providers,omitempty"`

TemplateStatusCommon `json:",inline"`
Expand All @@ -65,17 +66,14 @@ func (t *ServiceTemplate) FillStatusWithProviders(annotations map[string]string)
pspec, anno = t.Spec.Providers.InfrastructureProviders, ChartAnnotationInfraProviders
}

if len(pspec) > 0 {
return pspec
}

providers := annotations[anno]
if len(providers) == 0 {
return []string{}
return pspec
}

splitted := strings.Split(providers, ",")
splitted := strings.Split(providers, multiProviderSeparator)
result := make([]string, 0, len(splitted))
result = append(result, pspec...)
for _, v := range splitted {
if c := strings.TrimSpace(v); c != "" {
result = append(result, c)
Expand Down
13 changes: 7 additions & 6 deletions api/v1alpha1/templates_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,22 +85,23 @@ const (
infrastructureProvidersType
)

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)
if len(pspec) > 0 {
return pspec, nil
}

providers := annotations[anno]
if len(providers) == 0 {
return []ProviderTuple{}, nil
return pspec, nil
}

var (
splitted = strings.Split(providers, ",")
pstatus = make([]ProviderTuple, 0, len(splitted))
splitted = strings.Split(providers, multiProviderSeparator)
pstatus = make([]ProviderTuple, 0, len(splitted)+len(pspec))
merr error
)
pstatus = append(pstatus, pspec...)

for _, v := range splitted {
v = strings.TrimSpace(v)
nVerOrC := strings.SplitN(v, " ", 2)
Expand Down
1 change: 0 additions & 1 deletion api/v1alpha1/zz_generated.deepcopy.go

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

1 change: 0 additions & 1 deletion internal/controller/managedcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,6 @@ func (r *ManagedClusterReconciler) Update(ctx context.Context, managedCluster *h
}
// template is ok, propagate data from it
managedCluster.Status.KubertenesVersion = template.Status.KubertenesVersion
managedCluster.Status.Providers = template.Status.Providers

apimeta.SetStatusCondition(managedCluster.GetConditions(), metav1.Condition{
Type: hmc.TemplateReadyCondition,
Expand Down
8 changes: 4 additions & 4 deletions internal/controller/release_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@ type ReleaseReconciler struct {

Config *rest.Config

CreateManagement bool
CreateRelease bool
CreateTemplates bool

HMCTemplatesChartName string
SystemNamespace string

DefaultRegistryConfig helm.DefaultRegistryConfig

CreateManagement bool
CreateRelease bool
CreateTemplates bool
}

func (r *ReleaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
Expand Down
6 changes: 3 additions & 3 deletions internal/controller/template_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ const (
// TemplateReconciler reconciles a *Template object
type TemplateReconciler struct {
client.Client
SystemNamespace string

DefaultRegistryConfig helm.DefaultRegistryConfig

downloadHelmChartFunc func(context.Context, *sourcev1.Artifact) (*chart.Chart, error)

SystemNamespace string
DefaultRegistryConfig helm.DefaultRegistryConfig
}

type ClusterTemplateReconciler struct {
Expand Down
2 changes: 1 addition & 1 deletion internal/helm/release.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ type ReconcileHelmReleaseOpts struct {
OwnerReference *metav1.OwnerReference
ChartRef *hcv2.CrossNamespaceSourceReference
ReconcileInterval *time.Duration
DependsOn []meta.NamespacedObjectReference
TargetNamespace string
DependsOn []meta.NamespacedObjectReference
CreateNamespace bool
}

Expand Down
4 changes: 2 additions & 2 deletions internal/webhook/managedcluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,11 @@ func validateK8sCompatibility(ctx context.Context, cl client.Client, mc *hmcv1al

tplConstraint, err := semver.NewConstraint(kc)
if err != nil { // should never happen
return fmt.Errorf("failed to parse k8s constrainted version %s of the ServiceTemplate %s/%s: %w", kc, mc.Namespace, v.Template, err)
return fmt.Errorf("failed to parse k8s constrained version %s of the ServiceTemplate %s/%s: %w", kc, mc.Namespace, v.Template, err)
}

if !tplConstraint.Check(mcVersion) {
return fmt.Errorf("k8s version %s of the ManagedCluster %s/%s does not satisfy constrainted version %s from the ServiceTemplate %s/%s",
return fmt.Errorf("k8s version %s of the ManagedCluster %s/%s does not satisfy constrained version %s from the ServiceTemplate %s/%s",
mc.Status.KubertenesVersion, mc.Namespace, mc.Name,
kc, mc.Namespace, v.Template)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/webhook/managedcluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ one or more required infrastructure providers does not satisfy constraints: [aws
template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}),
),
},
err: fmt.Sprintf(`failed to validate k8s compatibility: k8s version v1.30.0 of the ManagedCluster default/managedcluster does not satisfy constrainted version <1.30 from the ServiceTemplate default/%s`, testTemplateName),
err: fmt.Sprintf(`failed to validate k8s compatibility: k8s version v1.30.0 of the ManagedCluster default/managedcluster does not satisfy constrained version <1.30 from the ServiceTemplate default/%s`, testTemplateName),
warnings: admission.Warnings{"Failed to validate k8s version compatibility with ServiceTemplates"},
},
}...)
Expand Down
89 changes: 85 additions & 4 deletions internal/webhook/management_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,17 @@ package webhook
import (
"context"
"errors"
"fmt"

apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

"github.com/Mirantis/hmc/api/v1alpha1"
"github.com/Masterminds/semver/v3"
hmcv1alpha1 "github.com/Mirantis/hmc/api/v1alpha1"
)

type ManagementValidator struct {
Expand All @@ -36,7 +39,7 @@ var errManagementDeletionForbidden = errors.New("management deletion is forbidde
func (v *ManagementValidator) SetupWebhookWithManager(mgr ctrl.Manager) error {
v.Client = mgr.GetClient()
return ctrl.NewWebhookManagedBy(mgr).
For(&v1alpha1.Management{}).
For(&hmcv1alpha1.Management{}).
WithValidator(v).
WithDefaulter(v).
Complete()
Expand All @@ -53,13 +56,91 @@ func (*ManagementValidator) ValidateCreate(_ context.Context, _ runtime.Object)
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
func (*ManagementValidator) ValidateUpdate(_ context.Context, _ runtime.Object, _ runtime.Object) (admission.Warnings, error) {
func (v *ManagementValidator) ValidateUpdate(ctx context.Context, _, newObj runtime.Object) (admission.Warnings, error) {
const invalidMgmtMsg = "the Management is invalid"

mgmt, ok := newObj.(*hmcv1alpha1.Management)
if !ok {
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected Management but got a %T", newObj))
}

if mgmt.Status.Release == "" {
return nil, nil // nothing to do at the moment
}

skipValidation := true
capiTplName := ""
for cn, c := range mgmt.Status.Components {
if cn == hmcv1alpha1.CoreCAPIName && c.Success {
skipValidation = false
capiTplName = c.Template
break
}
}

if skipValidation {
return nil, nil // either no status, or capi has been failed to deploy
}

ptpls := new(hmcv1alpha1.ProviderTemplateList)
if err := v.List(ctx, ptpls); err != nil {
return nil, fmt.Errorf("failed to list ProviderTemplates: %v", err)
}

if len(ptpls.Items) == 0 {
return nil, nil // nothing to do
}

name2Tpl := make(map[string]*hmcv1alpha1.ProviderTemplate, len(ptpls.Items)-1)
capiRequiredVersion := new(semver.Version)
for _, v := range ptpls.Items { // cluster-scoped
if v.Name != capiTplName {
name2Tpl[v.Name] = &v
continue
}

if v.Status.CAPIVersion == "" {
return nil, nil // nothing to validate against
}

var err error
capiRequiredVersion, err = semver.NewVersion(v.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, v.Status.CAPIVersion, v.Name, err)
}
}

var wrongVersions error
for _, c := range mgmt.Status.Components {
if c.Template == capiTplName {
continue
}

tpl, ok := name2Tpl[c.Template]
if !ok || tpl.Status.CAPIVersion == "" {
continue
}

ver, err := semver.NewVersion(tpl.Status.CAPIVersion)
if err != nil { // should never happen
return nil, fmt.Errorf("%s: invalid CAPI version %s in the ProviderTemplate %s: %v", invalidMgmtMsg, tpl.Status.CAPIVersion, tpl.Name, err)
}

if !capiRequiredVersion.Equal(ver) {
wrongVersions = errors.Join(wrongVersions, fmt.Errorf("wrong CAPI version in the ProviderTemplate %s: required %s but CAPI has %s", tpl.Name, ver, capiRequiredVersion))
}
}

if wrongVersions != nil {
return admission.Warnings{"The Management object has incompatible CAPI versions ProviderTemplates"}, fmt.Errorf("%s: %s", invalidMgmtMsg, wrongVersions)
}

return nil, nil
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
func (v *ManagementValidator) ValidateDelete(ctx context.Context, _ runtime.Object) (admission.Warnings, error) {
managedClusters := &v1alpha1.ManagedClusterList{}
managedClusters := &hmcv1alpha1.ManagedClusterList{}
err := v.Client.List(ctx, managedClusters, client.Limit(1))
if err != nil {
return nil, err
Expand Down
Loading

0 comments on commit aad600e

Please sign in to comment.