Fix numerous issues

* fix comment/naming (closes #754)
* comment isNonMajor, provide examples (closes #753)
* simplifie VerifyAPI (closes #758)
* enhance helmSpec String symbol (closes #756)
* simplified makeClientFromSecret (closes #762)
* renamed ArtifactReady and commented out (closes #764)
* added const for SA NS path in CurrentNamespace symbol (closes #772)
* isTemplateChainValid symbol: advice maps with the len (closes #774)
* validateReleaseWithValues symbol: simplify (closes #776)
* fix typo in test node name (closes #783)
* call linter from a single make target (closes #796)

Makefile

@@ -105,7 +105,7 @@ tidy:
 	go mod tidy
 
 .PHONY: test
-test: generate-all fmt vet envtest tidy external-crd ## Run tests.
+test: generate-all envtest tidy external-crd ## Run tests.
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out
 
 # Utilize Kind or modify the e2e tests to load the image locally, enabling
@@ -118,11 +118,11 @@ test-e2e: cli-install
 	KIND_CLUSTER_NAME="hmc-test" KIND_VERSION=$(KIND_VERSION) go test ./test/e2e/ -v -ginkgo.v -ginkgo.timeout=3h -timeout=3h $$ginkgo_label_flag
 
 .PHONY: lint
-lint: golangci-lint ## Run golangci-lint linter & yamllint
+lint: golangci-lint fmt vet ## Run golangci-lint linter & yamllint
 	@$(GOLANGCI_LINT) run --timeout=$(GOLANGCI_LINT_TIMEOUT)
 
 .PHONY: lint-fix
-lint-fix: golangci-lint ## Run golangci-lint linter and perform fixes
+lint-fix: golangci-lint fmt vet ## Run golangci-lint linter and perform fixes
 	@$(GOLANGCI_LINT) run --fix
 
 .PHONY: add-license
@@ -180,11 +180,11 @@ LD_FLAGS += -X github.com/Mirantis/hmc/internal/build.Version=$(VERSION)
 LD_FLAGS += -X github.com/Mirantis/hmc/internal/telemetry.segmentToken=$(SEGMENT_TOKEN)
 
 .PHONY: build
-build: generate-all fmt vet ## Build manager binary.
+build: generate-all ## Build manager binary.
 	go build -ldflags="${LD_FLAGS}" -o bin/manager cmd/main.go
 
 .PHONY: run
-run: generate-all fmt vet ## Run a controller from your host.
+run: generate-all ## Run a controller from your host.
 	go run ./cmd/main.go
 
 # If you wish to build the manager image targeting other platforms you can use the --platform flag.

api/v1alpha1/common.go

@@ -43,9 +43,10 @@ type (
 
 const (
 	// Provider CAPA
-	ProviderCAPAName = "cluster-api-provider-aws"
+	ProviderAWSName = "cluster-api-provider-aws"
 	// Provider Azure
-	ProviderAzureName   = "cluster-api-provider-azure"
+	ProviderAzureName = "cluster-api-provider-azure"
+	// Provider vSphere
 	ProviderVSphereName = "cluster-api-provider-vsphere"
 	// Provider K0smotron
 	ProviderK0smotronName = "k0smotron"

api/v1alpha1/compatibility_contract.go

@@ -61,6 +61,8 @@ func isCAPIContractSingleVersion(version string) bool {
 	return err == nil
 }
 
+// isNonMajor checks is a given version with "alpha" or "beta" version prefix
+// is a CAPI non-major version. Expects only ASCII chars, otherwise will return false (expected).
 func isNonMajor(version, prefix string, prefixIdx int) bool {
 	majorVer := version[:prefixIdx]
 	prefixedVer := version[prefixIdx+len(prefix):]

api/v1alpha1/compatibility_contract_test.go

@@ -14,7 +14,10 @@
 
 package v1alpha1
 
-import "testing"
+import (
+	"fmt"
+	"testing"
+)
 
 func Test_isCAPIContractVersion(t *testing.T) {
 	tests := []struct {
@@ -34,6 +37,8 @@ func Test_isCAPIContractVersion(t *testing.T) {
 		{"v1alpha", false},
 		{"v1beta", false},
 		{"v1alpha1beta1", false},
+		{"vNONSENSEalpha1beta1", false},
+		{"v©", false},
 	}
 
 	for _, test := range tests {
@@ -44,6 +49,19 @@ func Test_isCAPIContractVersion(t *testing.T) {
 	}
 }
 
+func Example_isNonMajor() {
+	_, _ = fmt.Printf("isNonMajor(\"1alpha1\", \"alpha\", 1): %v\n", isNonMajor("1alpha1", "alpha", 1))
+	_, _ = fmt.Printf("isNonMajor(\"1beta1\", \"beta\", 1): %v\n", isNonMajor("1beta1", "beta", 1))
+	_, _ = fmt.Printf("isNonMajor(\"NONSENSEbeta1\", \"beta\", 8): %v\n", isNonMajor("NONSENSEbeta1", "beta", 8))
+	_, _ = fmt.Printf("isNonMajor(\"beta1\", \"beta\", 1): %v\n", isNonMajor("beta1", "beta", 1))
+
+	// Output:
+	// isNonMajor("1alpha1", "alpha", 1): true
+	// isNonMajor("1beta1", "beta", 1): true
+	// isNonMajor("NONSENSEbeta1", "beta", 8): false
+	// isNonMajor("beta1", "beta", 1): false
+}
+
 func Test_isCAPIContractSingleVersion(t *testing.T) {
 	tests := []struct {
 		version string

api/v1alpha1/management_types.go

@@ -100,7 +100,7 @@ func (in *Component) HelmValues() (values map[string]any, err error) {
 func GetDefaultProviders() []Provider {
 	return []Provider{
 		{Name: ProviderK0smotronName},
-		{Name: ProviderCAPAName},
+		{Name: ProviderAWSName},
 		{Name: ProviderAzureName},
 		{Name: ProviderVSphereName},
 		{Name: ProviderSveltosName},

api/v1alpha1/templates_common.go

@@ -54,10 +54,18 @@ type HelmSpec struct {
 
 func (s *HelmSpec) String() string {
 	if s.ChartRef != nil {
-		return s.ChartRef.Namespace + "/" + s.ChartRef.Name + ", Kind=" + s.ChartRef.Kind
+		if s.ChartRef.Namespace != "" {
+			return s.ChartRef.Namespace + "/" + s.ChartRef.Name + ", Kind=" + s.ChartRef.Kind
+		}
+
+		return s.ChartRef.Name + ", Kind=" + s.ChartRef.Kind
+	}
+
+	if s.ChartSpec.Version != "" {
+		return s.ChartSpec.Chart + ": " + s.ChartSpec.Version
 	}
 
-	return s.ChartSpec.Chart + ": " + s.ChartSpec.Version
+	return s.ChartSpec.Chart
 }
 
 // TemplateStatusCommon defines the observed state of Template common for all Template types

internal/certmanager/verifier.go

@@ -26,9 +26,6 @@ func VerifyAPI(ctx context.Context, restcfg *rest.Config, namespace string) erro
 	if err != nil {
 		return err
 	}
-	err = checker.Check(ctx)
-	if err != nil {
-		return err
-	}
-	return nil
+
+	return checker.Check(ctx)
 }

internal/controller/accessmanagement_controller_test.go

@@ -163,7 +163,7 @@ var _ = Describe("Template Management Controller", func() {
 				Expect(k8sClient.Create(ctx, am)).To(Succeed())
 			}
 
-			By("creating custom resources for the Kind ClusterTemplateChain, ServiceTemplateChain amd Credentials")
+			By("creating custom resources for the Kind ClusterTemplateChain, ServiceTemplateChain and Credentials")
 			for _, obj := range []crclient.Object{
 				ctChain, ctChainToDelete, ctChainUnmanaged,
 				stChain, stChainToDelete, stChainUnmanaged,

internal/controller/managedcluster_controller.go

@@ -495,11 +495,9 @@ func validateReleaseWithValues(ctx context.Context, actionConfig *action.Configu
 	if err != nil {
 		return err
 	}
+
 	_, err = install.RunWithContext(ctx, hcChart, vals)
-	if err != nil {
-		return err
-	}
-	return nil
+	return err
 }
 
 // updateStatus updates the status for the ManagedCluster object.

internal/controller/template_controller.go

@@ -216,7 +216,7 @@ func (r *TemplateReconciler) ReconcileTemplate(ctx context.Context, template tem
 	}
 	status.ChartVersion = hcChart.Spec.Version
 
-	if reportStatus, err := helm.ArtifactReady(hcChart); err != nil {
+	if reportStatus, err := helm.ShouldReportStatusOnArtifactReadiness(hcChart); err != nil {
 		l.Info("HelmChart Artifact is not ready")
 		if reportStatus {
 			_ = r.updateStatus(ctx, template, err.Error())

internal/credspropagation/common.go

@@ -86,11 +86,7 @@ func makeClientFromSecret(kubeconfSecret *corev1.Secret) (client.Client, error)
 	if err != nil {
 		return nil, err
 	}
-	cl, err := client.New(restConfig, client.Options{
+	return client.New(restConfig, client.Options{
 		Scheme: scheme,
 	})
-	if err != nil {
-		return nil, err
-	}
-	return cl, nil
 }

internal/helm/chart.go

@@ -22,7 +22,9 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func ArtifactReady(chart *sourcev1.HelmChart) (reportStatus bool, _ error) {
+// ShouldReportStatusOnArtifactReadiness checks whether an artifact for the given chart is ready,
+// returns error and the flags, signaling if the caller should report the status.
+func ShouldReportStatusOnArtifactReadiness(chart *sourcev1.HelmChart) (bool, error) {
 	for _, c := range chart.Status.Conditions {
 		if c.Type == "Ready" {
 			if chart.Generation != c.ObservedGeneration {

internal/utils/kube.go

@@ -49,14 +49,20 @@ func EnsureDeleteAllOf(ctx context.Context, cl client.Client, gvk schema.GroupVe
 }
 
 func CurrentNamespace() string {
+	// Referencing https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/client-go/tools/clientcmd/client_config.go#L631-L646
+	// for simplicity
+
 	ns, found := os.LookupEnv("POD_NAMESPACE")
 	if found {
 		return ns
 	}
-	nsb, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace")
+
+	const serviceAccountNs = "/var/run/secrets/kubernetes.io/serviceaccount/namespace"
+	nsb, err := os.ReadFile(serviceAccountNs)
 	if err == nil && len(nsb) > 0 {
 		return string(nsb)
 	}
+
 	return DefaultSystemNamespace
 }
 

internal/webhook/management_webhook_test.go

@@ -106,7 +106,7 @@ func TestManagementValidateUpdate(t *testing.T) {
 	validStatus := v1alpha1.TemplateValidationStatus{Valid: true}
 
 	componentAwsDefaultTpl := v1alpha1.Provider{
-		Name: v1alpha1.ProviderCAPAName,
+		Name: v1alpha1.ProviderAWSName,
 		Component: v1alpha1.Component{
 			Template: template.DefaultName,
 		},

internal/webhook/templatechain_webhook.go

@@ -125,8 +125,8 @@ func (*ServiceTemplateChainValidator) Default(_ context.Context, _ runtime.Objec
 }
 
 func isTemplateChainValid(spec v1alpha1.TemplateChainSpec) admission.Warnings {
-	supportedTemplates := make(map[string]bool)
-	availableForUpgrade := make(map[string]bool)
+	supportedTemplates := make(map[string]bool, len(spec.SupportedTemplates))
+	availableForUpgrade := make(map[string]bool, len(spec.SupportedTemplates))
 	for _, supportedTemplate := range spec.SupportedTemplates {
 		supportedTemplates[supportedTemplate.Name] = true
 		for _, template := range supportedTemplate.AvailableUpgrades {