support gitlab drift detection #1936
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request adds support for GitLab CI alongside the existing GitHub CI detection. It introduces a new case in a switch statement to detect GitLab CI, logs an appropriate message, and invokes a new function from a newly created GitLab package. The new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DefaultCmd as defaultCmd.Run
participant Switch as CI Environment Switch
participant GitLabCI as gitlab.GitLabCI
participant API as Digger API
User->>DefaultCmd: Execute command
DefaultCmd->>Switch: Check CI environment
Switch-->>DefaultCmd: GitHub or GitLab detected
alt GitLab CI detected
DefaultCmd->>GitLabCI: Call GitLabCI(lock, policyChecker, backendApi, ...)
GitLabCI->>GitLabCI: Log "gitlab CI detected" and load environment
GitLabCI->>GitLabCI: Validate token and configuration
GitLabCI->>GitLabCI: If drift-detection mode, iterate projects
GitLabCI->>API: Execute drift detection job(s)
API-->>GitLabCI: Return job result
GitLabCI->>GitLabCI: Log success and completion
else GitHub CI detected
DefaultCmd->>/* GitHub Function */: (Handle GitHub CI)
end
GitLabCI-->>DefaultCmd: Return to main flow
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (8)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
This PR integrates GitLab drift detection support by modifying drift handling in the default command and adding a dedicated GitLab implementation.
- Added a new GitLab drift detection implementation in
/ee/cli/pkg/gitlab/gitlab.gothat gathers environment variables, loads config, and schedules drift jobs. - Modified
/ee/cli/cmd/digger/default.goto detect GitLab CI and invokegitlab.GitLabCI. - Updated
go.work.sumto include the new kong dependency checksum. - Noted issues: an unreachable defer block and an unused parameter in the GitLab implementation.
3 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
ee/cli/pkg/gitlab/gitlab.go
Outdated
| usage.ReportErrorAndExit(repoOwner, "Digger finished successfully", 0) | ||
|
|
||
| defer func() { | ||
| if r := recover(); r != nil { | ||
| usage.ReportErrorAndExit(repoOwner, fmt.Sprintf("Panic occurred. %s", r), 1) | ||
| } | ||
| }() |
There was a problem hiding this comment.
logic: Defer block placed after ReportErrorAndExit is unreachable. Consider moving defer above exit call.
| usage.ReportErrorAndExit(repoOwner, "Digger finished successfully", 0) | |
| defer func() { | |
| if r := recover(); r != nil { | |
| usage.ReportErrorAndExit(repoOwner, fmt.Sprintf("Panic occurred. %s", r), 1) | |
| } | |
| }() | |
| defer func() { | |
| if r := recover(); r != nil { | |
| usage.ReportErrorAndExit(repoOwner, fmt.Sprintf("Panic occurred. %s", r), 1) | |
| } | |
| }() | |
| usage.ReportErrorAndExit(repoOwner, "Digger finished successfully", 0) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
ee/cli/cmd/digger/default.go (2)
13-13: Add a clarifying package comment or rationale.
The new import forgitlabis straightforward; consider adding a short comment describing its role to ease future code navigation, especially for developers unfamiliar with the GitLab integration.
71-73: Ensure consistent logging and handling.
The new switch case fordigger.GitLabbehaves similarly toGitHub. Good work on maintaining consistency. One improvement: unify the logging style (e.g., uselog.Printf) and consider capturing the current actor or job context, as done for GitHub.ee/cli/pkg/gitlab/gitlab.go (2)
1-19: Unify import statements and logging approach.
You employ a mix offmt.Println,log.Printf, andprintlnthroughout this new file. Consider switching entirely tolog.Printffor easier log parsing and consistency across the codebase.
47-133: Consider modularizing for clarity and maintainability.
The single GitLabCI function is lengthy and includes environment parsing, config loading, drift detection logic, and job execution. Splitting it into smaller functions (e.g., for config loading vs. job orchestration) could enhance readability and reuse.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.work.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
ee/cli/cmd/digger/default.go(2 hunks)ee/cli/pkg/gitlab/gitlab.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
ee/cli/cmd/digger/default.go (2)
cli/pkg/digger/digger.go (1)
GitLab(37-37)ee/cli/pkg/gitlab/gitlab.go (1)
GitLabCI(21-133)
ee/cli/pkg/gitlab/gitlab.go (6)
libs/digger_config/digger_config.go (1)
LoadDiggerConfig(214-243)cli/pkg/usage/usage.go (1)
ReportErrorAndExit(111-118)libs/policy/policy.go (1)
NoOpPolicyChecker(31-32)libs/ci/gitlab/gitlab.go (2)
ParseGitLabContext(61-74)NewGitLabService(76-101)libs/scheduler/aws.go (1)
GetStateAndCommandProviders(364-387)cli/pkg/digger/digger.go (1)
RunJob(563-732)
🔇 Additional comments (1)
ee/cli/pkg/gitlab/gitlab.go (1)
21-46: Handle empty environment variables more robustly.
WhengitlabTokenis empty, you only print a message without terminating. If GitLab operations rely on the token, consider failing fast or explaining read-only scenarios. Similarly, consider validatingrepoOwnerorrepoNameto avoid unexpected nil references when constructingrepoFullName.
Summary by CodeRabbit
New Features
Dependency Updates