CNTRLPLANE-4008: enable hypershiftlinter and fix test naming

.golangci.yml

@@ -13,7 +13,12 @@ linters:
     - noctx
     - unparam
     - usestdlibvars
+    - hypershiftlinter
   settings:
+    custom:
+      hypershiftlinter:
+        path: hack/tools/bin/hypershiftlinter.so
+        description: "Enforces HyperShift test conventions from TESTING.md and test/e2e/v2/AGENTS.md"
     gocyclo:
       min-complexity: 30
     govet:

Makefile

@@ -103,7 +103,7 @@ $(KUBEAPILINTER_PLUGIN): $(TOOLS_DIR)/go.mod # Build kube-api-linter as Go plugi
 HYPERSHIFTLINTER_PLUGIN := $(abspath $(TOOLS_BIN_DIR)/hypershiftlinter.so)
 HYPERSHIFTLINTER_SRC := $(shell find $(TOOLS_DIR)/hypershiftlinter -name '*.go' 2>/dev/null)
 $(HYPERSHIFTLINTER_PLUGIN): $(TOOLS_DIR)/go.mod $(HYPERSHIFTLINTER_SRC) # Build hypershiftlinter as Go plugin
-	cd $(TOOLS_DIR); $(GO) build -a -buildmode=plugin -o $(HYPERSHIFTLINTER_PLUGIN) ./hypershiftlinter/cmd/plugin
+	cd $(TOOLS_DIR); CGO_ENABLED=1 $(GO) build -a -buildmode=plugin -o $(HYPERSHIFTLINTER_PLUGIN) ./hypershiftlinter/cmd/plugin
 
 # When not otherwise set, diff/lint against the upstream main branch.
 # This is always set in OpenShift CI.
@@ -123,21 +123,21 @@ precommit-api-lint-fix: $(GOLANGCI_LINT)
 	cd api && $(GOLANGCI_LINT) fmt --config ./.golangci.yml --enable gci $(patsubst api/%,%,$(FILES))
 
 .PHONY: lint
-lint: generate
+lint: generate $(HYPERSHIFTLINTER_PLUGIN)
 	$(MAKE) api-lint; api_rc=$$?; \
 	$(GOLANGCI_LINT) run --config ./.golangci.yml --modules-download-mode=readonly -v; main_rc=$$?; \
 	exit $$(( api_rc > main_rc ? api_rc : main_rc ))
 
 .PHONY: main-lint-fix
-main-lint-fix: generate $(GOLANGCI_LINT)
+main-lint-fix: generate $(GOLANGCI_LINT) $(HYPERSHIFTLINTER_PLUGIN)
 	$(GOLANGCI_LINT) run --config ./.golangci.yml --fix -v $(if $(PULL_BASE_SHA),--new-from-rev=$(PULL_BASE_SHA) --whole-files)
 
 .PHONY: precommit-main-lint-fix
 precommit-main-lint-fix: $(GOLANGCI_LINT)
 	$(GOLANGCI_LINT) fmt --config ./.golangci.yml --enable gci $(FILES)
 
 .PHONY: lint-fix
-lint-fix: generate
+lint-fix: generate $(HYPERSHIFTLINTER_PLUGIN)
 	$(MAKE) api-lint-fix; api_rc=$$?; \
 	$(GOLANGCI_LINT) run --config ./.golangci.yml --fix -v; main_rc=$$?; \
 	exit $$(( api_rc > main_rc ? api_rc : main_rc ))

api/karpenter/v1/kubelet_config_test.go

@@ -14,7 +14,7 @@ func TestKubeletConfigurationMarshalRoundTrip(t *testing.T) {
 		config KubeletConfiguration
 	}{
 		{
-			name: "When all typed fields are set they should round-trip",
+			name: "When all typed fields are set, it should round-trip",
 			config: KubeletConfiguration{
 				MaxPods:     110,
 				PodsPerCore: 10,
@@ -42,7 +42,7 @@ func TestKubeletConfigurationMarshalRoundTrip(t *testing.T) {
 			},
 		},
 		{
-			name: "When only some fields are set they should round-trip",
+			name: "When only some fields are set, it should round-trip",
 			config: KubeletConfiguration{
 				MaxPods:     50,
 				CPUCFSQuota: ptr.To(false),

cmd/cluster/agent/create_test.go

@@ -33,7 +33,7 @@ func TestCreateCluster(t *testing.T) {
 		args []string
 	}{
 		{
-			name: "minimal flags necessary to render",
+			name: "When minimal flags are provided, it should render successfully",
 			args: []string{
 				"--api-server-address=fakeAddress", // if we don't set it, the machine's IP is looked up, which isn't portable
 				"--render-sensitive",

cmd/cluster/aws/create_test.go

@@ -58,22 +58,22 @@ func TestValidateCreateCredentialInfo(t *testing.T) {
 		kubeconfigPath       string
 		expectError          bool
 	}{
-		"when CredentialSecretName is blank and aws-creds is also blank": {
+		"When CredentialSecretName and aws-creds are blank, it should return an error": {
 			expectError: true,
 		},
-		"when CredentialSecretName is blank, aws-creds is not blank, and pull-secret is blank": {
+		"When CredentialSecretName and pull-secret are blank and aws-creds is set, it should return an error": {
 			pullSecretFile:       "",
 			credentialSecretName: "",
 			credentials:          awsutil.AWSCredentialsOptions{AWSCredentialsFile: "asdf"},
 			expectError:          true,
 		},
-		"when CredentialSecretName is blank, aws-creds is not blank, and pull-secret is not blank": {
+		"When CredentialSecretName is blank and aws-creds and pull-secret are set, it should succeed": {
 			pullSecretFile:       "asdf",
 			credentialSecretName: "",
 			credentials:          awsutil.AWSCredentialsOptions{AWSCredentialsFile: "asdf"},
 			expectError:          false,
 		},
-		"when CredentialSecretName is set with invalid kubeconfig it should fail": {
+		"When CredentialSecretName is set with invalid kubeconfig, it should fail": {
 			credentialSecretName: "my-secret",
 			kubeconfigPath:       "/nonexistent/kubeconfig",
 			credentials:          awsutil.AWSCredentialsOptions{AWSCredentialsFile: "/some/creds"},
@@ -180,7 +180,7 @@ func TestCreateCluster(t *testing.T) {
 		args []string
 	}{
 		{
-			name: "minimal flags necessary to render",
+			name: "When minimal flags are provided, it should render successfully",
 			args: []string{
 				"--sts-creds=" + credentialsFile,
 				"--infra-json=" + infraFile,
@@ -192,7 +192,7 @@ func TestCreateCluster(t *testing.T) {
 			},
 		},
 		{
-			name: "default creation flags for cesar",
+			name: "When default creation flags are provided, it should create cluster with expected configuration",
 			args: []string{
 				"--pull-secret=" + pullSecretFile,
 				"--name=example",
@@ -215,7 +215,7 @@ func TestCreateCluster(t *testing.T) {
 			},
 		},
 		{
-			name: "minimal with KubeAPIServerDNSName",
+			name: "When KubeAPIServerDNSName is provided, it should configure custom DNS name",
 			args: []string{
 				"--name=example",
 				"--sts-creds=" + credentialsFile,
@@ -227,7 +227,7 @@ func TestCreateCluster(t *testing.T) {
 			},
 		},
 		{
-			name: "minimal with OVNKubernetesMTU",
+			name: "When OVNKubernetesMTU is provided, it should configure custom MTU",
 			args: []string{
 				"--name=example",
 				"--sts-creds=" + credentialsFile,

cmd/cluster/aws/destroy_test.go

@@ -9,12 +9,12 @@ import (
 	awsutil "github.com/openshift/hypershift/cmd/infra/aws/util"
 )
 
-func Test_ValidateCredentialInfo(t *testing.T) {
+func TestValidateCredentialInfo(t *testing.T) {
 	tests := map[string]struct {
 		inputOptions *core.DestroyOptions
 		expectError  bool
 	}{
-		"when CredentialSecretName is blank and aws-creds is also blank it should fall back to SDK default chain": {
+		"When CredentialSecretName is blank and aws-creds is also blank, it should fall back to SDK default chain": {
 			inputOptions: &core.DestroyOptions{
 				CredentialSecretName: "",
 				AWSPlatform: core.AWSPlatformDestroyOptions{
@@ -25,7 +25,7 @@ func Test_ValidateCredentialInfo(t *testing.T) {
 			},
 			expectError: false,
 		},
-		"when CredentialSecretName is blank and aws-creds is not blank": {
+		"When CredentialSecretName is blank and aws-creds is not blank, it should succeed": {
 			inputOptions: &core.DestroyOptions{
 				CredentialSecretName: "",
 				AWSPlatform: core.AWSPlatformDestroyOptions{
@@ -36,7 +36,7 @@ func Test_ValidateCredentialInfo(t *testing.T) {
 			},
 			expectError: false,
 		},
-		"when CredentialSecretName is set and AWSCredentialsFile is empty and RoleArn is empty it should fail": {
+		"When CredentialSecretName is set and AWSCredentialsFile is empty and RoleArn is empty, it should fail": {
 			inputOptions: &core.DestroyOptions{
 				CredentialSecretName: "my-secret",
 				AWSPlatform: core.AWSPlatformDestroyOptions{
@@ -48,7 +48,7 @@ func Test_ValidateCredentialInfo(t *testing.T) {
 			},
 			expectError: true,
 		},
-		"when CredentialSecretName is set and AWSCredentialsFile is not empty it should try to validate the secret": {
+		"When CredentialSecretName is set and AWSCredentialsFile is not empty, it should try to validate the secret": {
 			inputOptions: &core.DestroyOptions{
 				CredentialSecretName: "my-secret",
 				Kubeconfig:           "/nonexistent/kubeconfig",
@@ -60,7 +60,7 @@ func Test_ValidateCredentialInfo(t *testing.T) {
 			},
 			expectError: true,
 		},
-		"when CredentialSecretName is set and RoleArn is set it should try to validate the secret": {
+		"When CredentialSecretName is set and RoleArn is set, it should try to validate the secret": {
 			inputOptions: &core.DestroyOptions{
 				CredentialSecretName: "my-secret",
 				Kubeconfig:           "/nonexistent/kubeconfig",

cmd/cluster/azure/create_test.go

@@ -33,25 +33,25 @@ func TestValidateEndpointAccess(t *testing.T) {
 		expectError                                         bool
 		expectedErrorMsg                                    string
 	}{
-		"When endpoint-access has an invalid value it should return an error": {
+		"When endpoint-access has an invalid value, it should return an error": {
 			endpointAccess:   "InvalidValue",
 			expectError:      true,
 			expectedErrorMsg: "--endpoint-access must be one of: Public, PublicAndPrivate, Private",
 		},
-		"When endpoint-access is Private without nat-subnet-id it should succeed (controller auto-creates)": {
+		"When endpoint-access is Private without nat-subnet-id, it should succeed (controller auto-creates)": {
 			endpointAccess: "Private",
 			expectError:    false,
 		},
-		"When endpoint-access is PublicAndPrivate without nat-subnet-id it should succeed (controller auto-creates)": {
+		"When endpoint-access is PublicAndPrivate without nat-subnet-id, it should succeed (controller auto-creates)": {
 			endpointAccess: "PublicAndPrivate",
 			expectError:    false,
 		},
-		"When endpoint-access is Private with nat-subnet-id it should succeed without additional subscriptions": {
+		"When endpoint-access is Private with nat-subnet-id, it should succeed without additional subscriptions": {
 			endpointAccess:                   "Private",
 			endpointAccessPrivateNATSubnetID: "/subscriptions/sub-1/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/vnet/subnets/nat-subnet",
 			expectError:                      false,
 		},
-		"When endpoint-access is Public it should succeed without private connectivity fields": {
+		"When endpoint-access is Public, it should succeed without private connectivity fields": {
 			endpointAccess: "Public",
 			expectError:    false,
 		},
@@ -96,14 +96,14 @@ func TestDNSZoneRGValidation(t *testing.T) {
 		expectError bool
 		errContains string
 	}{
-		"When assign-service-principal-roles is set without dns-zone-rg-name it should return an error": {
+		"When assign-service-principal-roles is set without dns-zone-rg-name, it should return an error": {
 			extraArgs: []string{
 				"--assign-service-principal-roles",
 			},
 			expectError: true,
 			errContains: "--dns-zone-rg-name is required when --assign-service-principal-roles or --assign-custom-hcp-roles is set",
 		},
-		"When assign-custom-hcp-roles is set without dns-zone-rg-name it should return an error": {
+		"When assign-custom-hcp-roles is set without dns-zone-rg-name, it should return an error": {
 			extraArgs: []string{
 				"--assign-custom-hcp-roles",
 			},
@@ -210,22 +210,22 @@ func TestRoleAssignmentWithInfraJSON(t *testing.T) {
 		expectError bool
 		errContains string
 	}{
-		"When assign-custom-hcp-roles is set with infra-json it should return an error": {
+		"When assign-custom-hcp-roles is set with infra-json, it should return an error": {
 			extraArgs: []string{
 				"--assign-custom-hcp-roles",
 			},
 			expectError: true,
 			errContains: "role assignment flags cannot be used with --infra-json",
 		},
-		"When assign-service-principal-roles is set with infra-json it should return an error": {
+		"When assign-service-principal-roles is set with infra-json, it should return an error": {
 			extraArgs: []string{
 				"--assign-service-principal-roles",
 				"--dns-zone-rg-name=my-dns-rg",
 			},
 			expectError: true,
 			errContains: "role assignment flags cannot be used with --infra-json",
 		},
-		"When role assignment flags are not set with infra-json it should succeed": {
+		"When role assignment flags are not set with infra-json, it should succeed": {
 			extraArgs:   nil,
 			expectError: false,
 		},
@@ -283,7 +283,7 @@ func TestCreateCluster(t *testing.T) {
 		args []string
 	}{
 		{
-			name: "minimal flags necessary to render",
+			name: "When minimal flags are provided, it should render successfully",
 			args: []string{
 				"--azure-creds=" + credentialsFile,
 				"--infra-json=" + infraFile,
@@ -295,7 +295,7 @@ func TestCreateCluster(t *testing.T) {
 			},
 		},
 		{
-			name: "complicated invocation from bryan",
+			name: "When complex configuration flags are provided, it should create cluster with all options",
 			args: []string{
 				"--azure-creds=" + credentialsFile,
 				"--infra-json=" + infraFile,
@@ -314,7 +314,7 @@ func TestCreateCluster(t *testing.T) {
 			},
 		},
 		{
-			name: "create with azure marketplace image",
+			name: "When Azure Marketplace image flags are provided, it should configure marketplace image",
 			args: []string{
 				"--azure-creds=" + credentialsFile,
 				"--infra-json=" + infraFile,
@@ -336,7 +336,7 @@ func TestCreateCluster(t *testing.T) {
 			},
 		},
 		{
-			name: "with availability zones",
+			name: "When availability zones are provided, it should configure zones",
 			args: []string{
 				"--azure-creds=" + credentialsFile,
 				"--infra-json=" + infraFile,
@@ -349,7 +349,7 @@ func TestCreateCluster(t *testing.T) {
 			},
 		},
 		{
-			name: "with disabled capabilities",
+			name: "When disabled capabilities are provided, it should configure disabled capabilities",
 			args: []string{
 				"--name=example",
 				"--pull-secret=" + pullSecretFile,
@@ -362,7 +362,7 @@ func TestCreateCluster(t *testing.T) {
 			},
 		},
 		{
-			name: "with KubeAPIServerDNSName",
+			name: "When KubeAPIServerDNSName is provided, it should configure custom DNS name",
 			args: []string{
 				"--name=example",
 				"--pull-secret=" + pullSecretFile,
@@ -375,7 +375,7 @@ func TestCreateCluster(t *testing.T) {
 			},
 		},
 		{
-			name: "with image generation Gen1",
+			name: "When image generation Gen1 is provided, it should configure Gen1 images",
 			args: []string{
 				"--azure-creds=" + credentialsFile,
 				"--infra-json=" + infraFile,
@@ -387,7 +387,7 @@ func TestCreateCluster(t *testing.T) {
 			},
 		},
 		{
-			name: "with image generation Gen2",
+			name: "When image generation Gen2 is provided, it should configure Gen2 images",
 			args: []string{
 				"--azure-creds=" + credentialsFile,
 				"--infra-json=" + infraFile,
@@ -399,7 +399,7 @@ func TestCreateCluster(t *testing.T) {
 			},
 		},
 		{
-			name: "with marketplace flags and image generation Gen1",
+			name: "When marketplace flags and image generation Gen1 are provided, it should configure marketplace with Gen1",
 			args: []string{
 				"--azure-creds=" + credentialsFile,
 				"--infra-json=" + infraFile,
@@ -415,7 +415,7 @@ func TestCreateCluster(t *testing.T) {
 			},
 		},
 		{
-			name: "with availability zones and image generation Gen1",
+			name: "When availability zones and image generation Gen1 are provided, it should configure zones with Gen1",
 			args: []string{
 				"--azure-creds=" + credentialsFile,
 				"--infra-json=" + infraFile,
@@ -428,7 +428,7 @@ func TestCreateCluster(t *testing.T) {
 			},
 		},
 		{
-			name: "When endpoint-access is Private with endpoint-access-private flags it should render HostedCluster with Private endpoint access",
+			name: "When endpoint-access is Private with private flags, it should configure private endpoint access",
 			args: []string{
 				"--azure-creds=" + credentialsFile,
 				"--infra-json=" + infraFile,
@@ -443,7 +443,7 @@ func TestCreateCluster(t *testing.T) {
 			},
 		},
 		{
-			name: "When oauth-publishing-strategy is LoadBalancer with workload identities it should render HostedCluster with OAuth LoadBalancer",
+			name: "When oauth-publishing-strategy is LoadBalancer, it should configure OAuth LoadBalancer",
 			args: []string{
 				"--azure-creds=" + credentialsFile,
 				"--infra-json=" + infraFile,