diff --git a/.golangci.yml b/.golangci.yml index ae4c816ea..ee80ea33b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -9,31 +9,39 @@ issues: # restore some of the defaults # (fill in the rest as needed) exclude-rules: - - path: "internal/*" - linters: - - dupl - - text: "struct-tag: unknown option 'inline' in JSON tag" - linters: - - revive - - text: "Unhandled error in call to function fmt.Print*" - linters: - - revive - - path: cmd/main.go + - path: (cmd\/main\.go)|(.*_test\.go) linters: - maintidx - path: test/ linters: - perfsprint + - errorlint + - exhaustive + - text: 'shadow: declaration of "err" shadows declaration' + linters: + - govet + - text: "fieldalignment: " + path: (.*_test\.go|test\/|api\/?.*\/.*_types\.go) # ignore tests, and k8s API-specific files + linters: + - govet + - text: "max-public-structs: you have exceeded the maximum number of public struct declarations" + linters: + - revive + path: api/ # the api/ pkgs have lots of structs + linters: disable-all: true enable: + - asasalint - asciicheck + - bidichk - bodyclose - canonicalheader - containedctx - contextcheck - copyloopvar - decorder + - depguard - dogsled - dupl - dupword @@ -41,6 +49,8 @@ linters: - errcheck - errchkjson - errname + - errorlint + - exhaustive - forbidigo - forcetypeassert - gci @@ -89,10 +99,35 @@ linters: - whitespace linters-settings: + depguard: + rules: + # Name of a rule. + main: + # Packages that are not allowed where the value is a suggestion. + deny: + - pkg: "github.com/pkg/errors" + desc: Should be replaced by standard lib 'errors' package + - pkg: "github.com/hashicorp/go-multierror" + desc: Should be replaced by standard lib 'errors' package + - pkg: "go.uber.org/multierr" + desc: Should be replaced by standard lib 'errors' package + aliases: + # List of file globs that will match this list of settings to compare against. + # Default: $all + files: + - "!test/**" + - "!**/*_test.go" + deny: + - pkg: "sigs.k8s.io/controller-runtime/pkg/reconcile" + desc: Use aliases from the 'ctrl "sigs.k8s.io/controller-runtime"' package dupl: # Tokens count to trigger issue. # Default: 150 threshold: 200 + errcheck: + check-type-assertions: true + check-blank: false + disable-default-exclusions: true gci: sections: - standard # Standard section: captures all standard packages. @@ -109,10 +144,48 @@ linters-settings: gofumpt: extra-rules: true govet: - enable-all: true - disable: + disable-all: true + enable: + - appends + - asmdecl + - assign + - atomic + - atomicalign + - bools + - buildtag + - cgocall + - composites + - copylocks + - deepequalerrors + - defers + - directive + - errorsas - fieldalignment + - findcall + - framepointer + - httpresponse + - ifaceassert + - loopclosure + - lostcancel + - nilfunc + - nilness + - printf + - reflectvaluecompare - shadow + - shift + - sigchanyzer + - slog + - sortslice + - stdmethods + - stringintconv + - structtag + - testinggoroutine + - tests + - unmarshal + - unreachable + - unsafeptr + - unusedresult + - unusedwrite loggercheck: kitlog: false klog: false @@ -120,26 +193,138 @@ linters-settings: no-printf-like: true paralleltest: ignore-missing: true + prealloc: + for-loops: true revive: - enable-all-rules: true + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md rules: - name: add-constant disabled: true + - name: argument-limit + disabled: true + - name: atomic + - name: banned-characters + arguments: ["Ω", "Σ", "σ", "7"] + - name: bare-return + - name: blank-imports + - name: bool-literal-in-expr + - name: call-to-gc - name: cognitive-complexity disabled: true + - name: comment-spacings + - name: comments-density + disabled: true + - name: confusing-naming + - name: confusing-results + - name: constant-logical-expr + - name: context-as-argument + arguments: + - allowTypesBefore: "*testing.T" + - name: context-keys-type - name: cyclomatic disabled: true + - name: datarace + - name: deep-exit + - name: defer + arguments: + - ["call-chain", "loop", "method-call", "recover", "immediate-recover", "return"] - name: dot-imports - disabled: true + arguments: + - { allowedPackages: ["github.com/onsi/ginkgo/v2","github.com/onsi/gomega"] } + - name: duplicated-imports + - name: early-return + - name: empty-block + - name: empty-lines + - name: enforce-map-style + arguments: ["make"] + exclude: ["TEST"] + - name: enforce-repeated-arg-type-style + arguments: + - { funcArgStyle = "any", funcRetValStyle = "short" } + - name: enforce-slice-style + - name: error-naming + - name: error-return + - name: error-strings + - name: errorf - name: exported disabled: true + arguments: + - "checkPrivateReceivers" + - "sayRepetitiveInsteadOfStutters" + - name: file-header + disabled: true + - name: flag-parameter - name: function-length disabled: true - - name: max-public-structs - disabled: true # the api/* pkgs have lots of structs + - name: function-result-limit + disabled: true + - name: get-return + - name: identical-branches + - name: if-return + - name: import-alias-naming + arguments: + - "^[a-z][a-z0-9]{0,}$" + - name: import-shadowing + - name: imports-blocklist + disabled: true + - name: increment-decrement + - name: indent-error-flow - name: line-length-limit disabled: true + - name: max-control-nesting + - name: max-public-structs + exclude: ["TEST"] + arguments: [5] + - name: modifies-parameter + - name: modifies-value-receiver + - name: nested-structs + - name: optimize-operands-order - name: package-comments disabled: true + - name: range + - name: range-val-address + - name: range-val-in-closure + - name: receiver-naming + - name: redefines-builtin-id + - name: redundant-import-alias + - name: string-format + arguments: + # TODO: enable commented when existing warnings are fixed + # - - 'fmt.Errorf[0]' + # - '/^([^A-Z]|$)/' + # - 'Error string must not start with a capital letter.' + - - 'fmt.Errorf[0]' + - '/(^|[^\.!?])$/' + - 'Error string must not end in punctuation.' + # - - 'errors.New[0]' + # - '/^([^A-Z]|$)/' + # - 'Error string must not start with a capital letter.' + - - 'errors.New[0]' + - '/(^|[^\.!?])$/' + - 'Error string must not end in punctuation.' + - - 'panic' + - '/^[^\n]*$/' + - 'Must not contain line breaks.' + - name: string-of-int + - name: struct-tag + arguments: + - "json,inline" + - name: superfluous-else + - name: time-equal + - name: time-naming + - name: unchecked-type-assertion + - name: unconditional-recursion + - name: unexported-naming + - name: unexported-return + - name: unhandled-error + - name: unnecessary-stmt + - name: unreachable-code + - name: unused-parameter + - name: unused-receiver + - name: use-any + - name: useless-break + - name: var-declaration + - name: var-naming + - name: waitgroup-by-value stylecheck: checks: ["all", "-ST1000", "-ST1001", "-ST1021"] diff --git a/api/v1alpha1/clustertemplate_types.go b/api/v1alpha1/clustertemplate_types.go index 9d7c16b0f..d63bf9c5f 100644 --- a/api/v1alpha1/clustertemplate_types.go +++ b/api/v1alpha1/clustertemplate_types.go @@ -71,7 +71,7 @@ func (t *ClusterTemplate) FillStatusWithProviders(annotations map[string]string) contractsStatus, err := getCAPIContracts(t.Kind, t.Spec.ProviderContracts, annotations) if err != nil { - return fmt.Errorf("failed to get CAPI contract versions for ClusterTemplate %s/%s: %v", t.GetNamespace(), t.GetName(), err) + return fmt.Errorf("failed to get CAPI contract versions for ClusterTemplate %s/%s: %w", t.GetNamespace(), t.GetName(), err) } t.Status.ProviderContracts = contractsStatus diff --git a/api/v1alpha1/providertemplate_types.go b/api/v1alpha1/providertemplate_types.go index a9982f674..aadb122dd 100644 --- a/api/v1alpha1/providertemplate_types.go +++ b/api/v1alpha1/providertemplate_types.go @@ -50,7 +50,7 @@ func (t *ProviderTemplate) FillStatusWithProviders(annotations map[string]string contractsStatus, err := getCAPIContracts(t.Kind, t.Spec.CAPIContracts, annotations) if err != nil { - return fmt.Errorf("failed to get CAPI contract versions for ProviderTemplate %s: %v", t.GetName(), err) + return fmt.Errorf("failed to get CAPI contract versions for ProviderTemplate %s: %w", t.GetName(), err) } t.Status.CAPIContracts = contractsStatus diff --git a/internal/controller/managedcluster_controller.go b/internal/controller/managedcluster_controller.go index c8d5840a5..3e2e26d83 100644 --- a/internal/controller/managedcluster_controller.go +++ b/internal/controller/managedcluster_controller.go @@ -44,7 +44,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" - "sigs.k8s.io/controller-runtime/pkg/reconcile" hmc "github.com/Mirantis/hmc/api/v1alpha1" "github.com/Mirantis/hmc/internal/credspropagation" @@ -276,9 +275,9 @@ func (r *ManagedClusterReconciler) Update(ctx context.Context, managedCluster *h if !managedCluster.Spec.DryRun { helmValues, err := setIdentityHelmValues(managedCluster.Spec.Config, cred.Spec.IdentityRef) if err != nil { - return ctrl.Result{}, - fmt.Errorf("error setting identity values: %s", err) + return ctrl.Result{}, fmt.Errorf("error setting identity values: %w", err) } + hr, _, err := helm.ReconcileHelmRelease(ctx, r.Client, managedCluster.Name, managedCluster.Namespace, helm.ReconcileHelmReleaseOpts{ Values: helmValues, OwnerReference: &metav1.OwnerReference{ @@ -451,26 +450,23 @@ func (r *ManagedClusterReconciler) Delete(ctx context.Context, managedCluster *h l := ctrl.LoggerFrom(ctx) hr := &hcv2.HelmRelease{} - err := r.Get(ctx, client.ObjectKey{ - Name: managedCluster.Name, - Namespace: managedCluster.Namespace, - }, hr) - if err != nil { - if apierrors.IsNotFound(err) { - l.Info("Removing Finalizer", "finalizer", hmc.ManagedClusterFinalizer) - if controllerutil.RemoveFinalizer(managedCluster, hmc.ManagedClusterFinalizer) { - if err := r.Client.Update(ctx, managedCluster); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to update managedCluster %s/%s: %w", managedCluster.Namespace, managedCluster.Name, err) - } + + if err := r.Get(ctx, client.ObjectKeyFromObject(managedCluster), hr); err != nil { + if !apierrors.IsNotFound(err) { + return ctrl.Result{}, err + } + + l.Info("Removing Finalizer", "finalizer", hmc.ManagedClusterFinalizer) + if controllerutil.RemoveFinalizer(managedCluster, hmc.ManagedClusterFinalizer) { + if err := r.Client.Update(ctx, managedCluster); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to update managedCluster %s/%s: %w", managedCluster.Namespace, managedCluster.Name, err) } - l.Info("ManagedCluster deleted") - return ctrl.Result{}, nil } - return ctrl.Result{}, err + l.Info("ManagedCluster deleted") + return ctrl.Result{}, nil } - err = helm.DeleteHelmRelease(ctx, r.Client, managedCluster.Name, managedCluster.Namespace) - if err != nil { + if err := helm.DeleteHelmRelease(ctx, r.Client, managedCluster.Name, managedCluster.Namespace); err != nil { return ctrl.Result{}, err } @@ -479,13 +475,11 @@ func (r *ManagedClusterReconciler) Delete(ctx context.Context, managedCluster *h // It is detailed in https://github.com/projectsveltos/addon-controller/issues/732. // We may try to remove the explicit call to Delete once a fix for it has been merged. // TODO(https://github.com/Mirantis/hmc/issues/526). - err = sveltos.DeleteProfile(ctx, r.Client, managedCluster.Namespace, managedCluster.Name) - if err != nil { + if err := sveltos.DeleteProfile(ctx, r.Client, managedCluster.Namespace, managedCluster.Name); err != nil { return ctrl.Result{}, err } - err = r.releaseCluster(ctx, managedCluster.Namespace, managedCluster.Name, managedCluster.Spec.Template) - if err != nil { + if err := r.releaseCluster(ctx, managedCluster.Namespace, managedCluster.Name, managedCluster.Spec.Template); err != nil { return ctrl.Result{}, err } @@ -704,18 +698,16 @@ func setIdentityHelmValues(values *apiextensionsv1.JSON, idRef *corev1.ObjectRef var valuesJSON map[string]any err := json.Unmarshal(values.Raw, &valuesJSON) if err != nil { - return nil, fmt.Errorf("error unmarshalling values: %s", err) + return nil, fmt.Errorf("error unmarshalling values: %w", err) } valuesJSON["clusterIdentity"] = idRef valuesRaw, err := json.Marshal(valuesJSON) if err != nil { - return nil, fmt.Errorf("error marshalling values: %s", err) + return nil, fmt.Errorf("error marshalling values: %w", err) } - return &apiextensionsv1.JSON{ - Raw: valuesRaw, - }, nil + return &apiextensionsv1.JSON{Raw: valuesRaw}, nil } func (r *ManagedClusterReconciler) setAvailableUpgrades(ctx context.Context, managedCluster *hmc.ManagedCluster, template *hmc.ClusterTemplate) error { @@ -765,11 +757,8 @@ func (r *ManagedClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { if err != nil { return []ctrl.Request{} } - return []reconcile.Request{ - { - NamespacedName: managedClusterRef, - }, - } + + return []ctrl.Request{{NamespacedName: managedClusterRef}} }), ). Watches(&hmc.ClusterTemplateChain{}, diff --git a/internal/controller/management_controller.go b/internal/controller/management_controller.go index e2ade51c1..30f51663d 100644 --- a/internal/controller/management_controller.go +++ b/internal/controller/management_controller.go @@ -59,8 +59,8 @@ type ManagementReconciler struct { client.Client Scheme *runtime.Scheme Config *rest.Config - SystemNamespace string DynamicClient *dynamic.DynamicClient + SystemNamespace string CreateTemplateManagement bool } @@ -426,7 +426,7 @@ func (r *ManagementReconciler) enableAdditionalComponents(ctx context.Context, m if hmcComponent.Config != nil { err := json.Unmarshal(hmcComponent.Config.Raw, &config) if err != nil { - return fmt.Errorf("failed to unmarshal HMC config into map[string]any: %v", err) + return fmt.Errorf("failed to unmarshal HMC config into map[string]any: %w", err) } } @@ -452,7 +452,7 @@ func (r *ManagementReconciler) enableAdditionalComponents(ctx context.Context, m err := certmanager.VerifyAPI(ctx, r.Config, r.SystemNamespace) if err != nil { - return fmt.Errorf("failed to check in the cert-manager API is installed: %v", err) + return fmt.Errorf("failed to check in the cert-manager API is installed: %w", err) } l.Info("Cert manager is installed, enabling the HMC admission webhook") @@ -473,7 +473,7 @@ func (r *ManagementReconciler) enableAdditionalComponents(ctx context.Context, m updatedConfig, err := json.Marshal(config) if err != nil { - return fmt.Errorf("failed to marshal HMC config: %v", err) + return fmt.Errorf("failed to marshal HMC config: %w", err) } hmcComponent.Config = &apiextensionsv1.JSON{ Raw: updatedConfig, diff --git a/internal/controller/template_controller.go b/internal/controller/template_controller.go index edea1c72e..5b6aeb35a 100644 --- a/internal/controller/template_controller.go +++ b/internal/controller/template_controller.go @@ -237,7 +237,7 @@ func (r *TemplateReconciler) ReconcileTemplate(ctx context.Context, template tem helmChart, err := r.downloadHelmChartFunc(ctx, artifact) if err != nil { l.Error(err, "Failed to download Helm chart") - err = fmt.Errorf("failed to download chart: %s", err) + err = fmt.Errorf("failed to download chart: %w", err) _ = r.updateStatus(ctx, template, err.Error()) return ctrl.Result{}, err } @@ -261,7 +261,7 @@ func (r *TemplateReconciler) ReconcileTemplate(ctx context.Context, template tem rawValues, err := json.Marshal(helmChart.Values) if err != nil { l.Error(err, "Failed to parse Helm chart values") - err = fmt.Errorf("failed to parse Helm chart values: %s", err) + err = fmt.Errorf("failed to parse Helm chart values: %w", err) _ = r.updateStatus(ctx, template, err.Error()) return ctrl.Result{}, err } @@ -363,7 +363,7 @@ func (r *ClusterTemplateReconciler) validateCompatibilityAttrs(ctx context.Conte return err } - err = fmt.Errorf("failed to get Management: %v", err) + err = fmt.Errorf("failed to get Management: %w", err) _ = r.updateStatus(ctx, template, err.Error()) return err } diff --git a/internal/controller/templatechain_controller.go b/internal/controller/templatechain_controller.go index 093428585..5c749d21c 100644 --- a/internal/controller/templatechain_controller.go +++ b/internal/controller/templatechain_controller.go @@ -90,7 +90,7 @@ func (r *TemplateChainReconciler) ReconcileTemplateChain(ctx context.Context, te systemTemplates, managedTemplates, err := getCurrentTemplates(ctx, r.Client, templateChain.TemplateKind(), r.SystemNamespace, templateChain.GetNamespace(), templateChain.GetName()) if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to get current templates: %v", err) + return ctrl.Result{}, fmt.Errorf("failed to get current templates: %w", err) } var ( diff --git a/internal/controller/templatechain_controller_test.go b/internal/controller/templatechain_controller_test.go index a4813fd4c..68bfeaf7d 100644 --- a/internal/controller/templatechain_controller_test.go +++ b/internal/controller/templatechain_controller_test.go @@ -150,15 +150,12 @@ var _ = Describe("Template Chain Controller", func() { BeforeEach(func() { By("creating the system and test namespaces") for _, ns := range []string{namespace.Name, utils.DefaultSystemNamespace} { - namespace := &corev1.Namespace{} - err := k8sClient.Get(ctx, types.NamespacedName{Name: ns}, namespace) - if err != nil && errors.IsNotFound(err) { - namespace := &corev1.Namespace{ + if err := k8sClient.Get(ctx, types.NamespacedName{Name: ns}, &corev1.Namespace{}); errors.IsNotFound(err) { + Expect(k8sClient.Create(ctx, &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: ns, }, - } - Expect(k8sClient.Create(ctx, namespace)).To(Succeed()) + })).To(Succeed()) } } diff --git a/internal/credspropagation/azure.go b/internal/credspropagation/azure.go index 3d8fea730..05eb6973a 100644 --- a/internal/credspropagation/azure.go +++ b/internal/credspropagation/azure.go @@ -52,11 +52,11 @@ func PropagateAzureSecrets(ctx context.Context, cfg *PropagationCfg) error { ccmSecret, err := generateAzureCCMSecret(azureCluster, azureClIdty, azureSecret) if err != nil { - return fmt.Errorf("failed to generate Azure CCM secret: %s", err) + return fmt.Errorf("failed to generate Azure CCM secret: %w", err) } if err := applyCCMConfigs(ctx, cfg.KubeconfSecret, ccmSecret); err != nil { - return fmt.Errorf("failed to apply Azure CCM secret: %s", err) + return fmt.Errorf("failed to apply Azure CCM secret: %w", err) } return nil @@ -85,7 +85,7 @@ func generateAzureCCMSecret(azureCluster *capz.AzureCluster, azureClIdty *capz.A } azureJSON, err := json.Marshal(azureJSONMap) if err != nil { - return nil, fmt.Errorf("error marshalling azure.json: %s", err) + return nil, fmt.Errorf("error marshalling azure.json: %w", err) } secretData := map[string][]byte{ diff --git a/internal/credspropagation/vsphere.go b/internal/credspropagation/vsphere.go index d35459074..189a6bd32 100644 --- a/internal/credspropagation/vsphere.go +++ b/internal/credspropagation/vsphere.go @@ -70,15 +70,15 @@ func PropagateVSphereSecrets(ctx context.Context, cfg *PropagationCfg) error { } ccmSecret, ccmConfig, err := generateVSphereCCMConfigs(vsphereCluster, vsphereSecret, &vsphereMachines.Items[0]) if err != nil { - return fmt.Errorf("failed to generate VSphere CCM config: %s", err) + return fmt.Errorf("failed to generate VSphere CCM config: %w", err) } csiSecret, err := generateVSphereCSISecret(vsphereCluster, vsphereSecret, &vsphereMachines.Items[0]) if err != nil { - return fmt.Errorf("failed to generate VSphere CSI secret: %s", err) + return fmt.Errorf("failed to generate VSphere CSI secret: %w", err) } if err := applyCCMConfigs(ctx, cfg.KubeconfSecret, ccmSecret, ccmConfig, csiSecret); err != nil { - return fmt.Errorf("failed to apply VSphere CCM/CSI secrets: %s", err) + return fmt.Errorf("failed to apply VSphere CCM/CSI secrets: %w", err) } return nil @@ -113,7 +113,7 @@ func generateVSphereCCMConfigs(vCl *capv.VSphereCluster, vScrt *corev1.Secret, v ccmCfgYaml, err := yaml.Marshal(ccmCfg) if err != nil { - return nil, nil, fmt.Errorf("failed to marshal CCM config: %s", err) + return nil, nil, fmt.Errorf("failed to marshal CCM config: %w", err) } cmData := map[string]string{ @@ -150,11 +150,11 @@ datacenters = "{{ .Datacenter }}" tmpl, err := texttemplate.New("csiCfg").Parse(csiCfg) if err != nil { - return nil, fmt.Errorf("failed to generate CSI secret (tmpl parse): %s", err) + return nil, fmt.Errorf("failed to generate CSI secret (tmpl parse): %w", err) } var buf bytes.Buffer if err := tmpl.Execute(&buf, fields); err != nil { - return nil, fmt.Errorf("failed to generate CSI secret (tmpl execute): %s", err) + return nil, fmt.Errorf("failed to generate CSI secret (tmpl execute): %w", err) } secretData := map[string][]byte{ diff --git a/internal/webhook/managedcluster_webhook.go b/internal/webhook/managedcluster_webhook.go index 25970c588..5ad95f986 100644 --- a/internal/webhook/managedcluster_webhook.go +++ b/internal/webhook/managedcluster_webhook.go @@ -64,19 +64,19 @@ func (v *ManagedClusterValidator) ValidateCreate(ctx context.Context, obj runtim template, err := v.getManagedClusterTemplate(ctx, managedCluster.Namespace, managedCluster.Spec.Template) if err != nil { - return nil, fmt.Errorf("%s: %v", invalidManagedClusterMsg, err) + return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err) } if err := isTemplateValid(template); err != nil { - return nil, fmt.Errorf("%s: %v", invalidManagedClusterMsg, err) + return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err) } if err := validateK8sCompatibility(ctx, v.Client, template, managedCluster); err != nil { - return admission.Warnings{"Failed to validate k8s version compatibility with ServiceTemplates"}, fmt.Errorf("failed to validate k8s compatibility: %v", err) + return admission.Warnings{"Failed to validate k8s version compatibility with ServiceTemplates"}, fmt.Errorf("failed to validate k8s compatibility: %w", err) } if err := v.validateCredential(ctx, managedCluster, template); err != nil { - return nil, fmt.Errorf("%s: %v", invalidManagedClusterMsg, err) + return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err) } return nil, nil @@ -97,7 +97,7 @@ func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, oldObj, ne template, err := v.getManagedClusterTemplate(ctx, newManagedCluster.Namespace, newTemplate) if err != nil { - return nil, fmt.Errorf("%s: %v", invalidManagedClusterMsg, err) + return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err) } if oldTemplate != newTemplate { @@ -107,16 +107,16 @@ func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, oldObj, ne } if err := isTemplateValid(template); err != nil { - return nil, fmt.Errorf("%s: %v", invalidManagedClusterMsg, err) + return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err) } if err := validateK8sCompatibility(ctx, v.Client, template, newManagedCluster); err != nil { - return admission.Warnings{"Failed to validate k8s version compatibility with ServiceTemplates"}, fmt.Errorf("failed to validate k8s compatibility: %v", err) + return admission.Warnings{"Failed to validate k8s version compatibility with ServiceTemplates"}, fmt.Errorf("failed to validate k8s compatibility: %w", err) } } if err := v.validateCredential(ctx, newManagedCluster, template); err != nil { - return nil, fmt.Errorf("%s: %v", invalidManagedClusterMsg, err) + return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err) } return nil, nil @@ -182,11 +182,11 @@ func (v *ManagedClusterValidator) Default(ctx context.Context, obj runtime.Objec template, err := v.getManagedClusterTemplate(ctx, managedCluster.Namespace, managedCluster.Spec.Template) if err != nil { - return fmt.Errorf("could not get template for the managedcluster: %v", err) + return fmt.Errorf("could not get template for the managedcluster: %w", err) } if err := isTemplateValid(template); err != nil { - return fmt.Errorf("template is invalid: %v", err) + return fmt.Errorf("template is invalid: %w", err) } if template.Status.Config == nil { diff --git a/internal/webhook/management_webhook.go b/internal/webhook/management_webhook.go index e857eba54..16fee9b8c 100644 --- a/internal/webhook/management_webhook.go +++ b/internal/webhook/management_webhook.go @@ -122,7 +122,7 @@ func (v *ManagementValidator) ValidateUpdate(ctx context.Context, _, newObj runt } if wrongVersions != nil { - return admission.Warnings{"The Management object has incompatible CAPI contract versions in 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.Error()) } return nil, nil diff --git a/internal/webhook/templatemanagement_webhook.go b/internal/webhook/templatemanagement_webhook.go index 6025522bf..6e5f576f5 100644 --- a/internal/webhook/templatemanagement_webhook.go +++ b/internal/webhook/templatemanagement_webhook.go @@ -53,14 +53,16 @@ var ( // ValidateCreate implements webhook.Validator so a webhook will be registered for the type. func (v *TemplateManagementValidator) ValidateCreate(ctx context.Context, _ runtime.Object) (admission.Warnings, error) { itemsList := &metav1.PartialObjectMetadataList{} - gvk := v1alpha1.GroupVersion.WithKind(v1alpha1.TemplateManagementKind) - itemsList.SetGroupVersionKind(gvk) - if err := v.List(ctx, itemsList); err != nil { + itemsList.SetGroupVersionKind(v1alpha1.GroupVersion.WithKind(v1alpha1.TemplateManagementKind)) + + if err := v.List(ctx, itemsList, client.Limit(1)); err != nil { return nil, err } + if len(itemsList.Items) > 0 { return nil, errors.New("TemplateManagement object already exists") } + return nil, nil } @@ -72,18 +74,19 @@ func (*TemplateManagementValidator) ValidateUpdate(_ context.Context, _, _ runti // ValidateDelete implements webhook.Validator so a webhook will be registered for the type. func (v *TemplateManagementValidator) ValidateDelete(ctx context.Context, _ runtime.Object) (admission.Warnings, error) { partialList := &metav1.PartialObjectMetadataList{} - gvk := v1alpha1.GroupVersion.WithKind(v1alpha1.ManagementKind) - partialList.SetGroupVersionKind(gvk) - err := v.List(ctx, partialList) - if err != nil { - return nil, fmt.Errorf("failed to list Management objects: %v", err) + partialList.SetGroupVersionKind(v1alpha1.GroupVersion.WithKind(v1alpha1.ManagementKind)) + + if err := v.List(ctx, partialList, client.Limit(1)); err != nil { + return nil, fmt.Errorf("failed to list Management objects: %w", err) } + if len(partialList.Items) > 0 { mgmt := partialList.Items[0] if mgmt.DeletionTimestamp == nil { return nil, errTemplateManagementDeletionForbidden } } + return nil, nil } diff --git a/test/e2e/managedcluster/clusteridentity/clusteridentity.go b/test/e2e/managedcluster/clusteridentity/clusteridentity.go index dbdc342c4..41e01c77e 100644 --- a/test/e2e/managedcluster/clusteridentity/clusteridentity.go +++ b/test/e2e/managedcluster/clusteridentity/clusteridentity.go @@ -73,7 +73,7 @@ func New(kc *kubeclient.KubeClient, provider managedcluster.ProviderType) *Clust "secretRef": secretName, "allowedNamespaces": map[string]any{ "selector": map[string]any{ - "matchLabels": map[string]any{}, + "matchLabels": make(map[string]any), }, }, } @@ -85,7 +85,7 @@ func New(kc *kubeclient.KubeClient, provider managedcluster.ProviderType) *Clust "clientSecret": os.Getenv(managedcluster.EnvVarAzureClientSecret), } spec = map[string]any{ - "allowedNamespaces": map[string]any{}, + "allowedNamespaces": make(map[string]any), "clientID": os.Getenv(managedcluster.EnvVarAzureClientID), "clientSecret": map[string]any{ "name": secretName, @@ -107,7 +107,7 @@ func New(kc *kubeclient.KubeClient, provider managedcluster.ProviderType) *Clust "secretName": secretName, "allowedNamespaces": map[string]any{ "selector": map[string]any{ - "matchLabels": map[string]any{}, + "matchLabels": make(map[string]any), }, }, }