fix: a lot of sonar issue fixes#5771
Conversation
WalkthroughRemoves top-level GitHub Actions permissions in several workflows, adds per-job permissions where needed, adds explicit ref-resolution steps before checkouts in release workflows, centralizes a generator import path constant, refactors password generation and several provider implementations into helper functions/constants, and standardizes some provider error messages. Changes
Possibly related PRs
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
providers/v1/ibm/provider.go (1)
442-454: Refactor to use parseSecretReference and eliminate code duplication.This inline parsing logic duplicates the
parseSecretReferencefunction introduced earlier. This violates the DRY principle and creates a maintenance burden.🔎 Proposed refactor
func (ibm *providerIBM) GetSecretMap(_ context.Context, ref esv1.ExternalSecretDataRemoteRef) (map[string][]byte, error) { if esutils.IsNil(ibm.IBMClient) { return nil, errors.New(errUninitializedIBMProvider) } - var secretGroupName string - secretType := sm.Secret_SecretType_Arbitrary - secretName := ref.Key - nameSplitted := strings.Split(secretName, "/") - switch len(nameSplitted) { - case 2: - secretType = nameSplitted[0] - secretName = nameSplitted[1] - case 3: - secretGroupName = nameSplitted[0] - secretType = nameSplitted[1] - secretName = nameSplitted[2] - } + secretGroupName, secretType, secretName := parseSecretReference(ref.Key) secretMap := make(map[string][]byte) secMapBytes := make(map[string][]byte)
🧹 Nitpick comments (7)
cmd/esoctl/generator/bootstrap.go (1)
32-33: LGTM! Excellent refactoring to eliminate magic strings.Introducing the
generatorImportPathconstant provides a single source of truth for the generator import path, improving maintainability and reducing the risk of inconsistencies.generators/v1/password/password.go (3)
81-94: Excellent refactoring that improves code clarity.The simplified flow separates configuration extraction, validation, and generation into distinct steps, making the logic easier to understand and test.
96-104: Well-structured configuration grouping.The
passwordConfigstruct effectively consolidates all password generation parameters into a single cohesive unit, reducing parameter passing complexity and improving code organization.
153-170: Clean password generation logic.The function effectively separates the password generation concern, using the configuration struct to pass parameters and properly handling errors. This makes the code more testable and maintainable.
providers/v1/ibm/provider.go (2)
132-136: Remove unused type or refactor parseSecretReference to use it.The
secretIdentifiertype is defined but never used. TheparseSecretReferencefunction returns three separate strings instead of this structured type.🔎 Option 1: Remove the unused type
-type secretIdentifier struct { - secretGroupName string - secretType string - secretName string -} - func parseSecretReference(key string) (string, string, string) {🔎 Option 2: Refactor to use the type (preferred for better type safety)
-func parseSecretReference(key string) (string, string, string) { +func parseSecretReference(key string) secretIdentifier { var secretGroupName string secretType := sm.Secret_SecretType_Arbitrary secretName := key nameSplitted := strings.Split(key, "/") switch len(nameSplitted) { case 2: secretType = nameSplitted[0] secretName = nameSplitted[1] case 3: secretGroupName = nameSplitted[0] secretType = nameSplitted[1] secretName = nameSplitted[2] } - return secretGroupName, secretType, secretName + return secretIdentifier{ + secretGroupName: secretGroupName, + secretType: secretType, + secretName: secretName, + } }Then update the callers:
- secretGroupName, secretType, secretName := parseSecretReference(ref.Key) - return ibm.getSecretByType(secretType, secretName, secretGroupName, ref) + id := parseSecretReference(ref.Key) + return ibm.getSecretByType(id.secretType, id.secretName, id.secretGroupName, ref)
181-207: Consider consolidating validation wrappers to reduce duplication.The four validation wrapper functions follow an identical pattern. You could reduce duplication with a helper function.
🔎 Suggested consolidation
+func validatePropertyRequired(secretType string, ref esv1.ExternalSecretDataRemoteRef) error { + if ref.Property == "" { + return fmt.Errorf("remoteRef.property required for secret type %s", secretType) + } + return nil +} + func (ibm *providerIBM) getUsernamePasswordSecretWithValidation(secretName *string, ref esv1.ExternalSecretDataRemoteRef, secretGroupName string) ([]byte, error) { - if ref.Property == "" { - return nil, errors.New("remoteRef.property required for secret type username_password") + if err := validatePropertyRequired("username_password", ref); err != nil { + return nil, err } return getUsernamePasswordSecret(ibm, secretName, ref, secretGroupName) } func (ibm *providerIBM) getImportCertSecretWithValidation(secretName *string, ref esv1.ExternalSecretDataRemoteRef, secretGroupName string) ([]byte, error) { - if ref.Property == "" { - return nil, errors.New("remoteRef.property required for secret type imported_cert") + if err := validatePropertyRequired("imported_cert", ref); err != nil { + return nil, err } return getImportCertSecret(ibm, secretName, ref, secretGroupName) }(Apply similar pattern to the other two validation functions.)
.github/workflows/release.yml (1)
68-76: Consider extracting the ref resolution pattern into a reusable composite action.The identical ref resolution logic appears in both the
check-docs-for-releasejob (lines 38-46) and thereleasejob (lines 68-76), as well as in.github/workflows/release_esoctl.yml(lines 35-43). This duplication increases maintenance burden and the risk of inconsistent updates.You could create a composite action (e.g.,
.github/actions/resolve-ref/action.yml) that encapsulates the fetch, resolve, and checkout steps, then invoke it from each job. This would centralize the logic and make future improvements easier to apply consistently.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.github/workflows/ci.yml.github/workflows/helm.yml.github/workflows/pull-request-label.yml.github/workflows/release.yml.github/workflows/release_esoctl.yml.github/workflows/scorecard.yml.github/workflows/update-deps.ymlcmd/esoctl/generator/bootstrap.gogenerators/v1/password/password.goproviders/v1/cloudru/secretmanager/adapter/csm_client.goproviders/v1/fake/fake.goproviders/v1/github/client.goproviders/v1/ibm/provider.goproviders/v1/infisical/provider.go
💤 Files with no reviewable changes (3)
- .github/workflows/ci.yml
- .github/workflows/scorecard.yml
- .github/workflows/pull-request-label.yml
🧰 Additional context used
🧬 Code graph analysis (1)
generators/v1/password/password.go (1)
apis/generators/v1alpha1/types_password.go (1)
Password(77-82)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: publish-artifacts (Dockerfile.ubi, CGO_ENABLED=0 GOEXPERIMENT=boringcrypto, amd64 ppc64le, linux/... / Build and Publish
- GitHub Check: publish-artifacts (Dockerfile.ubi, CGO_ENABLED=0, amd64 arm64 ppc64le, linux/amd64,linux/arm64,li... / Build and Publish
- GitHub Check: publish-artifacts (Dockerfile, CGO_ENABLED=0, amd64 arm64 s390x ppc64le, linux/amd64,linux/arm64,... / Build and Publish
- GitHub Check: check-diff
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: Analyze project (go, autobuild)
- GitHub Check: Analyze project (actions, none)
🔇 Additional comments (16)
.github/workflows/update-deps.yml (1)
14-15: LGTM! Security improvement following the principle of least privilege.The addition of explicit per-job permissions with
contents: readfor the branches job is a security best practice. This job only needs to read repository content to set branch outputs, so the read-only permission is appropriate..github/workflows/helm.yml (1)
13-14: LGTM! Security improvement following the principle of least privilege.The addition of explicit per-job permissions with
contents: readfor the lint-and-test job is a security best practice. This job only needs to read repository content for linting and testing operations, so the read-only permission is appropriate.providers/v1/cloudru/secretmanager/adapter/csm_client.go (1)
38-39: LGTM! Error message centralization improves maintainability.Extracting the repeated "unauthorized: %w" error format into a constant reduces duplication and ensures consistency across all three authentication failure points (ListSecrets, AccessSecretVersionByPath, AccessSecretVersion). This aligns with DRY principles and addresses the code smell that SonarQube likely flagged.
Also applies to: 106-106, 122-122, 148-148
providers/v1/github/client.go (1)
35-36: LGTM! Consistent error messaging for unimplemented read operations.Centralizing the write-only provider error message into a constant eliminates duplication across the three unimplemented read methods (GetAllSecrets, GetSecret, GetSecretMap). This improves maintainability and addresses the SonarQube duplicate string literal issue.
Also applies to: 149-149, 154-154, 159-159
providers/v1/infisical/provider.go (4)
467-480: Excellent refactoring for improved maintainability.The extraction of validation logic into dedicated helper functions significantly improves code readability and reduces cognitive complexity. The error handling pattern is clean and consistent.
482-487: LGTM!The secrets scope validation correctly enforces that both
projectSlugandenvironmentSlugare required fields. The error message clearly communicates the requirement.
489-502: LGTM!The CA provider validation correctly enforces namespace constraints based on store type:
ClusterSecretStorerequires an explicit namespace for the CA provider referenceSecretStoremust not specify a namespace (inherits from store's namespace)The early return pattern and error messages are appropriate.
504-520: LGTM!The universal auth validation properly enforces:
- Secret selector validation (via
ValidateReferentSecretSelector) for namespace/scope constraints- Key presence validation to ensure both
clientIdandclientSecretkeys are populatedThe layered validation approach is appropriate and thorough.
cmd/esoctl/generator/bootstrap.go (1)
189-190: Consistent usage of the constant throughout the file.All occurrences of the hard-coded import path have been correctly replaced with
generatorImportPath, ensuring consistency across import line construction, string comparisons, and replace directive generation.Also applies to: 202-202, 328-329, 345-345, 361-361, 382-382
generators/v1/password/password.go (2)
106-134: Configuration extraction logic is well-implemented.The function correctly applies defaults and overrides them with spec values when provided. The logic preserves the original behavior while improving code organization.
136-151: Robust validation with proper error handling.The function correctly validates that keys are non-empty and unique, returning appropriate errors when validation fails. The default key assignment is a sensible fallback.
providers/v1/ibm/provider.go (3)
123-130: LGTM! Clean refactoring of secret retrieval.The delegation to
parseSecretReferenceandgetSecretByTypeimproves code organization and maintainability.
156-179: LGTM! Good centralization with proper error handling.The dispatcher consolidates type-based routing and adds support for
KvandCustomCredentialssecret types. The default case properly handles unknown secret types.
209-231: LGTM! Proper type handling for new secret types.Both handlers correctly retrieve secret data, perform type assertions with error checking, and delegate to the common
getKVOrCustomCredentialsSecrethelper.providers/v1/fake/fake.go (2)
143-154: LGTM!The refactored
GetAllSecretslogic is cleaner with the explicit nil check and delegation to helper methods. The two-step approach (collect matching secrets, then convert keys) improves readability.
156-169: LGTM!The helper correctly iterates over configured secrets, extracts the original key by trimming the version suffix (consistent with
mapKeyformat), and delegates version handling toupdateLatestVersion.
Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
|
/ok-to-test sha=7f2460038fb17e87374c45041f7821d7687bf38a |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
providers/v1/fake/fake.go (1)
171-177: Unused receiver on helper method.The receiver
(p *Provider)is not used within the method body. As noted in the previous review, consider making this a standalone function or removing the receiver if the provider instance isn't needed.🔎 Proposed refactor
-func (p *Provider) updateLatestVersion(originalKey string, data *Data, latestVersionMap map[string]string, dataMap map[string][]byte) { +func updateLatestVersion(originalKey string, data *Data, latestVersionMap map[string]string, dataMap map[string][]byte) { version, exists := latestVersionMap[originalKey] if !exists || version < data.Version { latestVersionMap[originalKey] = data.Version dataMap[originalKey] = []byte(data.Value) } }Then update the call on line 166:
- p.updateLatestVersion(originalKey, data, latestVersionMap, dataMap) + updateLatestVersion(originalKey, data, latestVersionMap, dataMap)
🧹 Nitpick comments (3)
providers/v1/ibm/provider.go (3)
175-201: Consider consolidating validation wrappers to reduce duplication.These four validation methods share identical structure—they all verify
ref.Propertyis non-empty before delegating to a handler. While the current implementation is correct, consolidating them could improve maintainability.💡 Optional refactor to reduce duplication
You could create a generic validation helper:
func (ibm *providerIBM) validateAndGetSecret( secretName *string, ref esv1.ExternalSecretDataRemoteRef, secretGroupName string, secretType string, handler func(*providerIBM, *string, esv1.ExternalSecretDataRemoteRef, string) ([]byte, error), ) ([]byte, error) { if ref.Property == "" { return nil, fmt.Errorf("remoteRef.property required for secret type %s", secretType) } return handler(ibm, secretName, ref, secretGroupName) }Then in
getSecretByType:case sm.Secret_SecretType_UsernamePassword: return ibm.validateAndGetSecret(secretName, ref, secretGroupName, "username_password", getUsernamePasswordSecret)
203-225: Consider consolidating KV and CustomCredentials handlers to reduce duplication.Both
getKVSecretandgetCustomCredentialsSecretfollow nearly identical patterns. While functionally correct, consolidation could improve maintainability.💡 Optional refactor to reduce duplication
Since both handlers share the same flow, you could create a generic helper that accepts a data extractor function:
func (ibm *providerIBM) getMapBasedSecret( secretName *string, secretGroupName string, secretType string, ref esv1.ExternalSecretDataRemoteRef, dataExtractor func(sm.SecretIntf) (map[string]interface{}, bool), ) ([]byte, error) { response, err := getSecretData(ibm, secretName, secretType, secretGroupName) if err != nil { return nil, err } data, ok := dataExtractor(response) if !ok { return nil, fmt.Errorf(errExtractingSecret, *secretName, secretType, "GetSecret") } return getKVOrCustomCredentialsSecret(ref, data) }Then call it with appropriate extractors for each type.
432-448: ReuseparseSecretReferenceto eliminate duplicate parsing logic.Lines 436-448 duplicate the parsing logic already extracted into
parseSecretReference(lines 132-148). Reusing the existing function would align with the DRY principle and maintain consistency.🔎 Proposed refactor
func (ibm *providerIBM) GetSecretMap(_ context.Context, ref esv1.ExternalSecretDataRemoteRef) (map[string][]byte, error) { if esutils.IsNil(ibm.IBMClient) { return nil, errors.New(errUninitializedIBMProvider) } - var secretGroupName string - secretType := sm.Secret_SecretType_Arbitrary - secretName := ref.Key - nameSplitted := strings.Split(secretName, "/") - switch len(nameSplitted) { - case 2: - secretType = nameSplitted[0] - secretName = nameSplitted[1] - case 3: - secretGroupName = nameSplitted[0] - secretType = nameSplitted[1] - secretName = nameSplitted[2] - } + secretGroupName, secretType, secretName := parseSecretReference(ref.Key) secretMap := make(map[string][]byte)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.github/workflows/ci.yml.github/workflows/helm.yml.github/workflows/pull-request-label.yml.github/workflows/release.yml.github/workflows/release_esoctl.yml.github/workflows/scorecard.yml.github/workflows/update-deps.ymlcmd/esoctl/generator/bootstrap.gogenerators/v1/password/password.goproviders/v1/cloudru/secretmanager/adapter/csm_client.goproviders/v1/fake/fake.goproviders/v1/github/client.goproviders/v1/ibm/provider.goproviders/v1/infisical/provider.go
💤 Files with no reviewable changes (3)
- .github/workflows/ci.yml
- .github/workflows/pull-request-label.yml
- .github/workflows/scorecard.yml
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/workflows/release.yml
- providers/v1/cloudru/secretmanager/adapter/csm_client.go
- .github/workflows/helm.yml
- .github/workflows/update-deps.yml
- providers/v1/github/client.go
🧰 Additional context used
🧬 Code graph analysis (3)
providers/v1/infisical/provider.go (1)
apis/externalsecrets/v1/secretsstore_infisical_types.go (2)
InfisicalProvider(156-178)UniversalAuthCredentials(24-29)
providers/v1/fake/fake.go (1)
providers/v1/infisical/provider.go (1)
Provider(53-58)
generators/v1/password/password.go (1)
apis/generators/v1alpha1/types_password.go (1)
Password(77-82)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: publish-artifacts (Dockerfile.ubi, CGO_ENABLED=0 GOEXPERIMENT=boringcrypto, amd64 ppc64le, linux/... / Build and Publish
- GitHub Check: publish-artifacts (Dockerfile.ubi, CGO_ENABLED=0, amd64 arm64 ppc64le, linux/amd64,linux/arm64,li... / Build and Publish
- GitHub Check: publish-artifacts (Dockerfile, CGO_ENABLED=0, amd64 arm64 s390x ppc64le, linux/amd64,linux/arm64,... / Build and Publish
- GitHub Check: lint
- GitHub Check: check-diff
- GitHub Check: unit-tests
- GitHub Check: Analyze project (go, autobuild)
🔇 Additional comments (18)
providers/v1/infisical/provider.go (4)
467-480: Excellent refactoring to improve maintainability.The extraction of validation logic into dedicated helper functions successfully reduces the cognitive complexity of
ValidateStoreand improves code organization. Each validator has a single, well-defined responsibility.
482-487: LGTM - Clear and focused validation.The function correctly validates that required
SecretsScopefields are not empty with a clear error message.
489-502: LGTM - Correct namespace validation for CA provider.The function correctly validates namespace requirements based on store kind, with early return for the nil case and clear error messages for each validation scenario.
504-520: LGTM - Thorough validation of Universal Auth credentials.The function correctly validates Universal Auth credentials by checking secret references and ensuring required keys are present.
.github/workflows/release_esoctl.yml (1)
35-43: Address the validation gaps identified in the previous review.The ref resolution pattern is a good security improvement that ensures the checked-out commit matches the intended reference. However, the previous review comment on these lines correctly identifies critical validation gaps that remain unaddressed:
RESOLVED_SHAcould be empty if bothgit rev-parseattempts fail- The
git fetchon line 38 will fail ifsource_refis a SHA or tag (rather than a branch)Please implement the suggested fixes from the previous review to add empty-SHA validation and handle fetch failures gracefully with
|| true.providers/v1/fake/fake.go (3)
143-145: Good defensive check for nilref.Name.The early nil check prevents potential nil pointer dereferences and provides a clear error message for unsupported find operators.
147-153: Clean refactoring that improves code organization.Extracting the secret collection logic into
collectMatchingSecretsimproves readability and testability. The flow is clear: create matcher, collect matching secrets, then convert keys.
156-169: Helper function correctly implements secret filtering and collection.The logic properly filters secrets using the matcher and delegates version tracking to
updateLatestVersion. The key extraction on line 161 correctly reverses the concatenation done inmapKey.providers/v1/ibm/provider.go (3)
128-130: LGTM! Clean refactoring improves separation of concerns.The extraction of parsing and dispatching logic makes
GetSecretmore maintainable and easier to understand.
132-148: LGTM! Parsing logic is correct and well-structured.The function correctly handles all three key format cases and provides sensible defaults.
150-173: LGTM! Dispatcher pattern improves maintainability and adds new secret type support.The switch-based dispatcher clearly routes each secret type to its handler. The addition of
KvandCustomCredentialssecret types expands functionality, and the default case provides helpful error messages for unknown types.cmd/esoctl/generator/bootstrap.go (3)
32-33: LGTM! Centralizes generator import path.Introducing the
generatorImportPathconstant eliminates hardcoded string duplication and provides a single source of truth for the generator import path. This improves maintainability and aligns with DRY principles.
189-190: LGTM! Correct usage of the constant.The constant is properly used to construct the import line for the new generator package.
202-202: LGTM! Consistent usage across different contexts.The constant is correctly used throughout the file:
- With quote prefix for import statement detection (line 202)
- Without quotes for go.mod operations (lines 328-330, 345, 361, 382)
The logic properly handles the different contexts where the path appears.
Also applies to: 328-330, 345-345, 361-361, 382-382
generators/v1/password/password.go (4)
82-94: LGTM! Excellent refactoring that reduces complexity.The refactored
generatefunction now delegates to well-defined helper functions:
- Configuration extraction
- Key validation
- Password generation
This improves readability, testability, and reduces cognitive complexity—likely addressing SonarQube complexity metrics.
96-134: LGTM! Clean configuration extraction with proper defaults.The
passwordConfigstruct andextractPasswordConfigfunction properly encapsulate all password generation parameters. The logic correctly applies defaults first, then user-specified overrides, maintaining the original behavior while improving modularity.
136-151: LGTM! Proper validation with sensible defaults.The
validateSecretKeysfunction correctly:
- Applies the default key "password" when none are specified
- Validates that keys are non-empty
- Ensures uniqueness using a map
The validation logic is sound and maintains the original behavior.
153-170: LGTM! Clean password generation with proper error handling.The
generatePasswordsfunction properly generates a password for each key using the provided configuration and generator function. The use of a function parameter (passGen) enables easier testing with mock implementations.
Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/release_esoctl.yml (1)
35-51: Consider validating RESOLVED_SHA after assignment to handle edge cases.The ref resolution logic is sound for the common case (branch names). However, there's an edge case where if
git fetchsucceeds (line 40) for a tag butgit rev-parse "origin/<tag>"fails (line 42), the script will exit due toset -ewithout reaching the clear error message in the else block. Tags don't haveorigin/tracking refs, so the rev-parse would fail.In practice, this is unlikely to cause issues since tags should be available locally after the
fetch-depth: 0checkout, and the elif branch should handle them. However, for defensive coding, consider adding a validation check after setting RESOLVED_SHA:🔎 Optional improvement for edge case handling
- name: Resolve and validate ref id: resolve_ref run: | set -e # Try to fetch the ref from remote if git fetch origin "${{ github.event.inputs.source_ref }}"; then # Remote ref exists, use it - RESOLVED_SHA=$(git rev-parse "origin/${{ github.event.inputs.source_ref }}") + RESOLVED_SHA=$(git rev-parse "origin/${{ github.event.inputs.source_ref }}" 2>/dev/null || echo "") elif git rev-parse --verify "${{ github.event.inputs.source_ref }}" >/dev/null 2>&1; then # Local ref exists (e.g., a tag) RESOLVED_SHA=$(git rev-parse "${{ github.event.inputs.source_ref }}") else echo "Error: ref '${{ github.event.inputs.source_ref }}' not found" exit 1 fi + if [ -z "$RESOLVED_SHA" ]; then + echo "Error: Unable to resolve ref '${{ github.event.inputs.source_ref }}' to a SHA" + exit 1 + fi echo "Resolved to SHA: $RESOLVED_SHA" echo "sha=$RESOLVED_SHA" >> $GITHUB_OUTPUT
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release_esoctl.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: publish-artifacts (Dockerfile.ubi, CGO_ENABLED=0, amd64 arm64 ppc64le, linux/amd64,linux/arm64,li... / Build and Publish
- GitHub Check: publish-artifacts (Dockerfile.ubi, CGO_ENABLED=0 GOEXPERIMENT=boringcrypto, amd64 ppc64le, linux/... / Build and Publish
- GitHub Check: publish-artifacts (Dockerfile, CGO_ENABLED=0, amd64 arm64 s390x ppc64le, linux/amd64,linux/arm64,... / Build and Publish
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: check-diff
- GitHub Check: Analyze project (go, autobuild)
- GitHub Check: Analyze project (actions, none)
🔇 Additional comments (1)
.github/workflows/release_esoctl.yml (1)
53-54: LGTM!The checkout step correctly uses the validated SHA from the previous step. This approach ensures that the workflow operates on a known, immutable commit rather than a potentially moving ref.
|
Problem Statement
What is the problem you're trying to solve?
Related Issue
Fixes #...
Proposed Changes
How do you like to solve the issue and why?
Format
Please ensure that your PR follows the following format for the title:
Where
scopeis optionally one of:Checklist
git commit --signoffmake testmake reviewableFixes multiple Sonar/code-quality issues across the project.
GitHub Actions
Go refactors and small fixes
Other
Overall: reduces duplication, improves maintainability, and addresses static-analysis findings.