pkg/providers/azure/azure.go:158

• [nitpick] The variable name 'vmp' is ambiguous; a more descriptive name, such as 'vmProviderInstance', could improve code clarity.

vmp := &vmProvider{Authorizer: p.Authorizer, SubscriptionID: subscriptionID, id: p.id}

pkg/providers/azure/azure.go:168

• [nitpick] The variable name 'publicIPp' is unclear; renaming it to a more descriptive identifier like 'publicIPProviderInstance' could enhance readability.

publicIPp := &publicIPProvider{Authorizer: p.Authorizer, SubscriptionID: subscriptionID, id: p.id}

.gitignore (1)

3-5: Ensure sensitive and generated files remain untracked.
The addition of .env and provider-config.yaml to the ignore list is a good security practice to prevent accidental commits of sensitive configuration data. Including the dist directory explicitly also helps avoid tracking build artifacts.

Consider restoring a trailing newline at the end of this file to comply with POSIX standards and common formatting conventions.

pkg/providers/azure/azure.go (1)

155-178: Consider parallelizing resource fetching for each subscription.
Iterating subscriptions sequentially works but may be slow under high subscription counts. Parallelizing calls or add concurrency controls might optimize performance.

pkg/providers/gcp/gcp.go (1)

207-253: ⚠️ Potential issue

The GCP verification implementation has a logic issue.

While the overall approach is sound, the implementation has an issue - it will return an error as soon as the first attempted service check fails, rather than trying all services first. This differs from the AWS provider's approach.

The code should continue checking services even if an earlier one fails. Apply this fix:

-			if err != nil {
-				return errorutil.NewWithErr(err).Msgf("failed to verify compute service access")
-			}
-			success = true
+			if err == nil {
+				success = true
+			}

Make similar changes to the other service checks in this method.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

// Verify checks if the GCP provider credentials are valid
func (p *Provider) Verify(ctx context.Context) error {
	if len(p.projects) == 0 {
		return errorutil.New("no accessible GCP projects found with provided credentials")
	}

	// For extra verification, try a minimal API call on one service
	for _, project := range p.projects {
		var success bool
		if p.compute != nil {
			_, err := p.compute.Regions.List(project).Do()
			if err == nil {
				success = true
			}
		} else if p.dns != nil {
			_, err := p.dns.ManagedZones.List(project).Do()
			if err == nil {
				success = true
			}
		} else if p.storage != nil {
			_, err := p.storage.Buckets.List(project).Do()
			if err == nil {
				success = true
			}
		} else if p.functions != nil {
			_, err := p.functions.Projects.Locations.List(project).Do()
			if err == nil {
				success = true
			}
		} else if p.run != nil {
			_, err := p.run.Projects.Locations.List(project).Do()
			if err == nil {
				success = true
			}
		}
		// For any one service to be successful, we can return nil
		if success {
			return nil
		}
	}
	return errorutil.New("no accessible GCP services found with provided credentials")
}

pkg/providers/azure/azure.go (1)

183-212: ⚠️ Potential issue

Unconditional return ends the verification loop prematurely.
The loop over multiple subscriptions never proceeds beyond the first iteration. Lines 208 and 210 each return from the function, causing a static analysis warning (SA4004). Unless you only want to verify the first subscription, this prevents checking subsequent subscriptions.

Below is a suggested fix that checks each subscription and returns success only if at least one subscription is accessible:

 func (p *Provider) Verify(ctx context.Context) error {
-    for _, subscriptionID := range p.SubscriptionIDs {
-        ...
-        if success {
-            return nil
-        }
-        return fmt.Errorf("no accessible Azure services found with provided credentials")
-    }
-    return fmt.Errorf("no accessible Azure services found with provided credentials")
+    var foundAtLeastOne bool
+    for _, subscriptionID := range p.SubscriptionIDs {
+        var success bool
+        // attempt to verify "vm" or "publicip" services...
+        // if verification passes:
+        //   success = true
+
+        if success {
+            foundAtLeastOne = true
+            // optionally break here if only one valid subscription is required
+        }
+    }
+    if foundAtLeastOne {
+        return nil
+    }
+    return fmt.Errorf("no accessible Azure services found with provided credentials")
 }

Decide if you want to validate all subscriptions or terminate upon the first valid subscription, then adjust accordingly.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

// Verify checks if the provider is valid using simple API call
func (p *Provider) Verify(ctx context.Context) error {
	var foundAtLeastOne bool
	for _, subscriptionID := range p.SubscriptionIDs {
		groupsClient := resources.NewGroupsClient(subscriptionID)
		groupsClient.Authorizer = p.Authorizer

		pClient := network.NewPublicIPAddressesClient(subscriptionID)
		pClient.Authorizer = p.Authorizer

		// Try a lightweight operation - just list the first group
		var success bool
		if p.services.Has("vm") {
			_, err := groupsClient.List(ctx, "", nil)
			if err != nil {
				return fmt.Errorf("failed to verify Azure credentials: %v", err)
			}
			success = true
		} else if p.services.Has("publicip") && !success {
			_, err := pClient.ListAllComplete(ctx)
			if err != nil {
				return fmt.Errorf("failed to verify Azure credentials: %v", err)
			}
			success = true
		}

		if success {
			foundAtLeastOne = true
			// optionally break here if only one valid subscription is required
		}
	}
	if foundAtLeastOne {
		return nil
	}
	return fmt.Errorf("no accessible Azure services found with provided credentials")
}

🧰 Tools

🪛 golangci-lint (1.64.8)

210-210: SA4004: the surrounding loop is unconditionally terminated

(staticcheck)

pkg/providers/azure/azure.go (1)

185-211: Verification short-circuits on the first successful subscription.
If you want a more comprehensive verification of all accessible subscriptions, consider continuing through each one. Currently, once a single subscription’s credentials are verified, the method returns success.

pkg/providers/gcp/dns.go (1)

69-69: Clarify the linter rule being ignored.
It’s often useful to specify which linter or rule is being disabled, for instance //nolint:gocyclo, to help understand the intention.

pkg/providers/terraform/state_file.go (1)

30-34: Improved error handling for file closure, but error message needs correction.

The change to properly handle and log file closure errors is excellent. However, the error message refers to a "provider config file" when this is actually closing a Terraform state file.

- gologger.Error().Msgf("Could not close provider config file: %s\n", err)
+ gologger.Error().Msgf("Could not close Terraform state file: %s\n", err)

pkg/providers/custom/services.go (1)

43-47: Improved error handling for response body closure, but error message needs correction.

The change to properly handle and log response body closure errors is excellent. However, the error message refers to a "provider config file" when this is actually closing an HTTP response body.

- gologger.Error().Msgf("Could not close provider config file: %s\n", err)
+ gologger.Error().Msgf("Could not close HTTP response body: %s\n", err)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)