Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for GitLab groups #4001

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions cmd/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ const (
GiteaUserFlag = "gitea-user"
GiteaWebhookSecretFlag = "gitea-webhook-secret" // nolint: gosec
GiteaPageSizeFlag = "gitea-page-size"
GitlabGroupAllowlistFlag = "gitlab-group-allowlist"
GitlabHostnameFlag = "gitlab-hostname"
GitlabTokenFlag = "gitlab-token"
GitlabUserFlag = "gitlab-user"
Expand Down Expand Up @@ -358,6 +359,17 @@ var stringFlags = map[string]stringFlag{
"This means that an attacker could spoof calls to Atlantis and cause it to perform malicious actions. " +
"Should be specified via the ATLANTIS_GITEA_WEBHOOK_SECRET environment variable.",
},
GitlabGroupAllowlistFlag: {
description: "Comma separated list of key-value pairs representing the GitLab groups and the operations that " +
"the members of a particular group are allowed to perform. " +
"The format is {group}:{command},{group}:{command}. " +
"Valid values for 'command' are 'plan', 'apply' and '*', e.g. 'myorg/dev:plan,myorg/ops:apply,myorg/devops:*'" +
"This example gives the users from the 'myorg/dev' GitLab group the permissions to execute the 'plan' command, " +
"the 'myorg/ops' group the permissions to execute the 'apply' command, " +
"and allows the 'myorg/devops' group to perform any operation. If this argument is not provided, the default value (*:*) " +
"will be used and the default behavior will be to not check permissions " +
"and to allow users from any group to perform any operation.",
},
GitlabHostnameFlag: {
description: "Hostname of your GitLab Enterprise installation. If using gitlab.com, no need to set.",
defaultValue: DefaultGitlabHostname,
Expand Down
1 change: 1 addition & 0 deletions cmd/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ var testFlags = map[string]interface{}{
GiteaUserFlag: "gitea-user",
GiteaWebhookSecretFlag: "gitea-secret",
GiteaPageSizeFlag: 30,
GitlabGroupAllowlistFlag: "",
GitlabHostnameFlag: "gitlab-hostname",
GitlabTokenFlag: "gitlab-token",
GitlabUserFlag: "gitlab-user",
Expand Down
16 changes: 16 additions & 0 deletions runatlantis.io/docs/server-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,22 @@ based on the organization or user that triggered the webhook.
This means that an attacker could spoof calls to Atlantis and cause it to perform malicious actions.
:::

### `--gitlab-group-allowlist`

```bash
atlantis server --gitlab-group-allowlist="myorg/mygroup:plan, myorg/secteam:apply, myorg/devops:apply, myorg/devops:import"
# or
ATLANTIS_GITLAB_GROUP_ALLOWLIST="myorg/mygroup:plan, myorg/secteam:apply, myorg/devops:apply, myorg/devops:import"
```

Comma-separated list of GitLab groups and permission pairs.

By default, any group can plan and apply.

::: warning NOTE
Atlantis needs to be able to view the listed group members, inaccessible or non-existent groups are silently ignored.
:::

### `--gitlab-hostname`

```bash
Expand Down
12 changes: 6 additions & 6 deletions runatlantis.io/docs/server-side-repo-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -592,12 +592,12 @@ mode: on_apply

### Policies

| Key | Type | Default | Required | Description |
|------------------------|-----------------|---------|-----------|----------------------------------------------------------|
| conftest_version | string | none | no | conftest version to run all policy sets |
| owners | Owners(#Owners) | none | yes | owners that can approve failing policies |
| approve_count | int | 1 | no | number of approvals required to bypass failing policies. |
| policy_sets | []PolicySet | none | yes | set of policies to run on a plan output |
| Key | Type | Default | Required | Description |
|------------------------|-----------------|---------|-----------|---------------------------------------------------------|
| conftest_version | string | none | no | conftest version to run all policy sets |
| owners | Owners(#Owners) | none | yes | owners that can approve failing policies |
| approve_count | int | 1 | no | number of approvals required to bypass failing policies |
| policy_sets | []PolicySet | none | yes | set of policies to run on a plan output |

### Owners

Expand Down
14 changes: 14 additions & 0 deletions server/core/config/valid/policies.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package valid

import (
"slices"
"strings"

version "github.com/hashicorp/go-version"
Expand Down Expand Up @@ -67,3 +68,16 @@ func (o *PolicyOwners) IsOwner(username string, userTeams []string) bool {

return false
}

// Return all owner teams from all policy sets
func (p *PolicySets) AllTeams() []string {
teams := p.Owners.Teams
for _, policySet := range p.PolicySets {
for _, team := range policySet.Owners.Teams {
if !slices.Contains(teams, team) {
teams = append(teams, team)
}
}
}
return teams
}
63 changes: 63 additions & 0 deletions server/core/config/valid/policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,3 +120,66 @@ func TestPoliciesConfig_IsOwners(t *testing.T) {
})
}
}

func TestPoliciesConfig_AllTeams(t *testing.T) {
cases := []struct {
description string
input valid.PolicySets
expResult []string
}{
{
description: "has only top-level team owner",
input: valid.PolicySets{
Owners: valid.PolicyOwners{
Teams: []string{
"team1",
},
},
},
expResult: []string{"team1"},
},
{
description: "has only policy-level team owner",
input: valid.PolicySets{
PolicySets: []valid.PolicySet{
{
Name: "policy1",
Owners: valid.PolicyOwners{
Teams: []string{
"team2",
},
},
},
},
},
expResult: []string{"team2"},
},
{
description: "has both top-level and policy-level team owners",
input: valid.PolicySets{
Owners: valid.PolicyOwners{
Teams: []string{
"team1",
},
},
PolicySets: []valid.PolicySet{
{
Name: "policy1",
Owners: valid.PolicyOwners{
Teams: []string{
"team2",
},
},
},
},
},
expResult: []string{"team1", "team2"},
},
}
for _, c := range cases {
t.Run(c.description, func(t *testing.T) {
result := c.input.AllTeams()
Equals(t, c.expResult, result)
})
}
}
14 changes: 14 additions & 0 deletions server/events/command/team_allowlist_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ type TeamAllowlistChecker interface {

// IsCommandAllowedForAnyTeam determines if any of the specified teams can perform the specified action
IsCommandAllowedForAnyTeam(ctx models.TeamAllowlistCheckerContext, teams []string, command string) bool

// AllTeams returns all teams configured in the allowlist
AllTeams() []string
}

// DefaultTeamAllowlistChecker implements checking the teams and the operations that the members
Expand Down Expand Up @@ -84,3 +87,14 @@ func (checker *DefaultTeamAllowlistChecker) IsCommandAllowedForAnyTeam(ctx model
}
return false
}

// AllTeams returns all teams configured in the allowlist
func (checker *DefaultTeamAllowlistChecker) AllTeams() []string {
var teamNames []string
for _, rule := range checker.rules {
for key := range rule {
teamNames = append(teamNames, key)
}
}
return teamNames
}
8 changes: 4 additions & 4 deletions server/events/command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo

// Check if the user who triggered the autoplan has permissions to run 'plan'.
if c.TeamAllowlistChecker != nil && c.TeamAllowlistChecker.HasRules() {
err := c.fetchUserTeams(baseRepo, &user)
err := c.fetchUserTeams(log, baseRepo, &user)
if err != nil {
log.Err("Unable to fetch user teams: %s", err)
return
Expand Down Expand Up @@ -300,7 +300,7 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead

// Check if the user who commented has the permissions to execute the 'plan' or 'apply' commands
if c.TeamAllowlistChecker != nil && c.TeamAllowlistChecker.HasRules() {
err := c.fetchUserTeams(baseRepo, &user)
err := c.fetchUserTeams(log, baseRepo, &user)
if err != nil {
c.Logger.Err("Unable to fetch user teams: %s", err)
return
Expand Down Expand Up @@ -491,8 +491,8 @@ func (c *DefaultCommandRunner) ensureValidRepoMetadata(
return
}

func (c *DefaultCommandRunner) fetchUserTeams(repo models.Repo, user *models.User) error {
teams, err := c.VCSClient.GetTeamNamesForUser(repo, *user)
func (c *DefaultCommandRunner) fetchUserTeams(logger logging.SimpleLogging, repo models.Repo, user *models.User) error {
teams, err := c.VCSClient.GetTeamNamesForUser(logger, repo, *user)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions server/events/command_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ func TestRunCommentCommand_TeamAllowListChecker(t *testing.T) {
When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(&pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil)

ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan})
vcsClient.VerifyWasCalled(Never()).GetTeamNamesForUser(testdata.GithubRepo, testdata.User)
vcsClient.VerifyWasCalled(Never()).GetTeamNamesForUser(ch.Logger, testdata.GithubRepo, testdata.User)
vcsClient.VerifyWasCalledOnce().CreateComment(
Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(modelPull.Num), Eq("Ran Plan for 0 projects:"), Eq("plan"))
})
Expand All @@ -331,7 +331,7 @@ func TestRunCommentCommand_TeamAllowListChecker(t *testing.T) {
When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(&pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil)

ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan})
vcsClient.VerifyWasCalled(Never()).GetTeamNamesForUser(testdata.GithubRepo, testdata.User)
vcsClient.VerifyWasCalled(Never()).GetTeamNamesForUser(ch.Logger, testdata.GithubRepo, testdata.User)
vcsClient.VerifyWasCalledOnce().CreateComment(
Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(modelPull.Num), Eq("Ran Plan for 0 projects:"), Eq("plan"))
})
Expand Down
4 changes: 4 additions & 0 deletions server/events/external_team_allowlist_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ func (checker *ExternalTeamAllowlistChecker) IsCommandAllowedForAnyTeam(ctx mode
return checker.checkOutputResults(out)
}

func (checker *ExternalTeamAllowlistChecker) AllTeams() []string {
return []string{}
}

func (checker *ExternalTeamAllowlistChecker) buildCommandString(ctx models.TeamAllowlistCheckerContext, teams []string, command string) string {
// Build command string
// Format is "$external_cmd $external_args $command $repo $teams"
Expand Down
5 changes: 2 additions & 3 deletions server/events/mock_workingdir_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion server/events/project_command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ type DefaultProjectCommandRunner struct {
VcsClient vcs.Client
Locker ProjectLocker
LockURLGenerator LockURLGenerator
Logger logging.SimpleLogging
InitStepRunner StepRunner
PlanStepRunner StepRunner
ShowStepRunner StepRunner
Expand Down Expand Up @@ -367,7 +368,7 @@ func (p *DefaultProjectCommandRunner) doApprovePolicies(ctx command.ProjectConte
// Only query the users team membership if any teams have been configured as owners on any policy set(s).
if policySetCfg.HasTeamOwners() {
// A convenient way to access vcsClient. Not sure if best way.
userTeams, err := p.VcsClient.GetTeamNamesForUser(ctx.Pull.BaseRepo, ctx.User)
userTeams, err := p.VcsClient.GetTeamNamesForUser(p.Logger, ctx.Pull.BaseRepo, ctx.User)
if err != nil {
ctx.Log.Err("unable to get team membership for user: %s", err)
return nil, "", err
Expand Down
2 changes: 1 addition & 1 deletion server/events/project_command_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1276,7 +1276,7 @@ func TestDefaultProjectCommandRunner_ApprovePolicies(t *testing.T) {
}

modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num, Author: testdata.User.Username}
When(runner.VcsClient.GetTeamNamesForUser(testdata.GithubRepo, testdata.User)).ThenReturn(c.userTeams, nil)
When(runner.VcsClient.GetTeamNamesForUser(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.User))).ThenReturn(c.userTeams, nil)
ctx := command.ProjectContext{
User: testdata.User,
Log: logging.NewNoopLogger(t),
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/azuredevops_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ func SplitAzureDevopsRepoFullName(repoFullName string) (owner string, project st
}

// GetTeamNamesForUser returns the names of the teams or groups that the user belongs to (in the organization the repository belongs to).
func (g *AzureDevopsClient) GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) { //nolint: revive
func (g *AzureDevopsClient) GetTeamNamesForUser(_ logging.SimpleLogging, _ models.Repo, _ models.User) ([]string, error) { //nolint: revive
return nil, nil
}

Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/bitbucketcloud/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ func (b *Client) makeRequest(method string, path string, reqBody io.Reader) ([]b
}

// GetTeamNamesForUser returns the names of the teams or groups that the user belongs to (in the organization the repository belongs to).
func (b *Client) GetTeamNamesForUser(_ models.Repo, _ models.User) ([]string, error) {
func (b *Client) GetTeamNamesForUser(_ logging.SimpleLogging, _ models.Repo, _ models.User) ([]string, error) {
return nil, nil
}

Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/bitbucketserver/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ func (b *Client) makeRequest(method string, path string, reqBody io.Reader) ([]b
}

// GetTeamNamesForUser returns the names of the teams or groups that the user belongs to (in the organization the repository belongs to).
func (b *Client) GetTeamNamesForUser(_ models.Repo, _ models.User) ([]string, error) {
func (b *Client) GetTeamNamesForUser(_ logging.SimpleLogging, _ models.Repo, _ models.User) ([]string, error) {
return nil, nil
}

Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type Client interface {
DiscardReviews(repo models.Repo, pull models.PullRequest) error
MergePull(logger logging.SimpleLogging, pull models.PullRequest, pullOptions models.PullRequestOptions) error
MarkdownPullLink(pull models.PullRequest) (string, error)
GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error)
GetTeamNamesForUser(logger logging.SimpleLogging, repo models.Repo, user models.User) ([]string, error)

// GetFileContent a repository file content from VCS (which support fetch a single file from repository)
// The first return value indicates whether the repo contains a file or not
Expand Down
2 changes: 1 addition & 1 deletion server/events/vcs/gitea/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ func (c *GiteaClient) MarkdownPullLink(pull models.PullRequest) (string, error)
}

// GetTeamNamesForUser returns the names of the teams or groups that the user belongs to (in the organization the repository belongs to).
func (c *GiteaClient) GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) {
func (c *GiteaClient) GetTeamNamesForUser(_ logging.SimpleLogging, _ models.Repo, _ models.User) ([]string, error) {
// TODO: implement
return nil, errors.New("GetTeamNamesForUser not (yet) implemented for Gitea client")
}
Expand Down
3 changes: 2 additions & 1 deletion server/events/vcs/github_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1009,7 +1009,8 @@ func (g *GithubClient) MarkdownPullLink(pull models.PullRequest) (string, error)

// GetTeamNamesForUser returns the names of the teams or groups that the user belongs to (in the organization the repository belongs to).
// https://docs.github.com/en/graphql/reference/objects#organization
func (g *GithubClient) GetTeamNamesForUser(repo models.Repo, user models.User) ([]string, error) {
func (g *GithubClient) GetTeamNamesForUser(logger logging.SimpleLogging, repo models.Repo, user models.User) ([]string, error) {
logger.Debug("Getting GitHub team names for user '%s'", user)
orgName := repo.Owner
variables := map[string]interface{}{
"orgName": githubv4.String(orgName),
Expand Down
12 changes: 7 additions & 5 deletions server/events/vcs/github_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1397,11 +1397,13 @@ func TestGithubClient_GetTeamNamesForUser(t *testing.T) {
Ok(t, err)
defer disableSSLVerification()()

teams, err := client.GetTeamNamesForUser(models.Repo{
Owner: "testrepo",
}, models.User{
Username: "testuser",
})
teams, err := client.GetTeamNamesForUser(
logger,
models.Repo{
Owner: "testrepo",
}, models.User{
Username: "testuser",
})
Ok(t, err)
Equals(t, []string{"Frontend Developers", "frontend-developers", "Employees", "employees"}, teams)
}
Expand Down
Loading
Loading