test token change #1965
test token change #1965
Conversation
WalkthroughThe Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Graph Analysis (1)ee/cli/pkg/policy/providers.go (3)
🪛 golangci-lint (1.64.8)ee/cli/pkg/policy/providers.go17-17: ineffectual assignment to token (ineffassign) 18-18: ineffectual assignment to tokenName (ineffassign) ⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Identified inconsistencies in token handling within the policy provider implementation.
- Error message in
ee/cli/pkg/policy/providers.goincorrectly references 'GITHUB_TOKEN' when checking for 'GITLAB_TOKEN' - Inconsistent token usage between
GetPolicyProviderandPolicyCheckerProviderAdvanced.Getmethods - Missing structured logging with relevant context for error cases
- Token validation logic needs to be unified across both methods for consistent behavior
1 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
ee/cli/pkg/policy/providers.go
Outdated
| token := os.Getenv("GITLAB_TOKEN") | ||
| if token == "" { | ||
| return nil, fmt.Errorf("failed to get managent repo policy provider: GITHUB_TOKEN not specified") | ||
| } |
There was a problem hiding this comment.
logic: Error message references GITHUB_TOKEN but checks for GITLAB_TOKEN. Update error message to match the token being checked.
| token := os.Getenv("GITLAB_TOKEN") | |
| if token == "" { | |
| return nil, fmt.Errorf("failed to get managent repo policy provider: GITHUB_TOKEN not specified") | |
| } | |
| token := os.Getenv("GITLAB_TOKEN") | |
| if token == "" { | |
| return nil, fmt.Errorf("failed to get managent repo policy provider: GITLAB_TOKEN not specified") | |
| } |
| @@ -14,7 +14,7 @@ func (p AdvancedPolicyProvider) GetPolicyProvider(policySpec lib_spec.PolicySpec | |||
| managementRepo := os.Getenv("DIGGER_MANAGEMENT_REPO") | |||
| if managementRepo != "" { | |||
| log.Printf("info: using management repo policy provider") | |||
There was a problem hiding this comment.
style: Log message should include management repo URL for better debugging context
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ee/cli/pkg/policy/providers.go (1)
38-40: Verify token consistency and fix typo.There's an inconsistency between this method and
AdvancedPolicyProvider.GetPolicyProvider- this method still usesGITHUB_TOKENwhile the other was changed toGITLAB_TOKEN. Please verify if this inconsistency is intentional or if both methods should use the same token type.Also, there's a typo in the error message: "managent" should be "management".
Apply this diff to fix the typo:
- return nil, fmt.Errorf("failed to get managent repo policy provider: GITHUB_TOKEN not specified") + return nil, fmt.Errorf("failed to get management repo policy provider: GITHUB_TOKEN not specified")Please verify if this method should also be updated to use
GITLAB_TOKENfor consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ee/cli/pkg/policy/providers.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: build-and-push-image
- GitHub Check: binary
- GitHub Check: build-and-push-image
- GitHub Check: build-and-push-image
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
ee/cli/pkg/policy/providers.go
Outdated
| token := os.Getenv("GITLAB_TOKEN") | ||
| if token == "" { | ||
| return nil, fmt.Errorf("failed to get managent repo policy provider: GITHUB_TOKEN not specified") |
There was a problem hiding this comment.
Fix inconsistent error message after environment variable change.
The code now retrieves GITLAB_TOKEN but the error message still references GITHUB_TOKEN, which will confuse users during debugging. Additionally, there's a typo in "managent".
Apply this diff to fix the inconsistency and typo:
- return nil, fmt.Errorf("failed to get managent repo policy provider: GITHUB_TOKEN not specified")
+ return nil, fmt.Errorf("failed to get management repo policy provider: GITLAB_TOKEN not specified")📝 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.
| token := os.Getenv("GITLAB_TOKEN") | |
| if token == "" { | |
| return nil, fmt.Errorf("failed to get managent repo policy provider: GITHUB_TOKEN not specified") | |
| token := os.Getenv("GITLAB_TOKEN") | |
| if token == "" { | |
| return nil, fmt.Errorf("failed to get management repo policy provider: GITLAB_TOKEN not specified") | |
| } |
🤖 Prompt for AI Agents
In ee/cli/pkg/policy/providers.go around lines 17 to 19, the error message
incorrectly references GITHUB_TOKEN instead of GITLAB_TOKEN and contains a typo
in "managent". Update the error message to correctly say "failed to get
management repo policy provider: GITLAB_TOKEN not specified" to fix both the
environment variable name and the typo.
Summary by CodeRabbit