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

Add Server Side YAML Restrictions #486

Closed
wants to merge 1 commit into from

Conversation

jjulien
Copy link
Contributor

@jjulien jjulien commented Feb 20, 2019

Added new --allow-restricted-repo-config and --repo-config options

This allows usage of a server side repo config file to restrict certain fields from being used in a repos atlantis.yaml file.

When this feature is activated apply_requirements, workflow, and workflows can only be specified in an atlantis.yaml file if explicitly allowed by the server side repo config.

The repo config file provides the ability to specify a default set of workflows, and default values for apply_requirements and workflow to use use on a per repo basis. It also supports applying to a collection of repos by using regex to match a repo name.

If more than one repo name matches, the values from last repo matched are used.

Closes #47

@codecov
Copy link

codecov bot commented Feb 20, 2019

Codecov Report

Merging #486 into master will increase coverage by 0.53%.
The diff coverage is 94.7%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #486      +/-   ##
==========================================
+ Coverage   72.18%   72.72%   +0.53%     
==========================================
  Files          62       63       +1     
  Lines        4516     4641     +125     
==========================================
+ Hits         3260     3375     +115     
- Misses       1012     1017       +5     
- Partials      244      249       +5
Impacted Files Coverage Δ
server/user_config.go 100% <ø> (ø) ⬆️
server/events/models/models.go 86.15% <100%> (+0.55%) ⬆️
server/events/yaml/parser_validator.go 99.22% <100%> (+0.73%) ⬆️
server/events/yaml/raw/repo_config.go 100% <100%> (ø)
cmd/server.go 77.5% <50%> (-1.45%) ⬇️
server/server.go 67.98% <77.77%> (+0.32%) ⬆️
server/events/project_command_builder.go 83.26% <93.75%> (-1.43%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5dfcfb...9819e5f. Read the comment docs.

@jjulien jjulien force-pushed the iss_47_server_yaml branch 3 times, most recently from d312079 to 9854e2f Compare February 23, 2019 05:02
@lkysow
Copy link
Member

lkysow commented Feb 25, 2019

Hi John, thanks for the amazing PR! I'm hoping to get some time this week to review.

@kipkoan
Copy link
Contributor

kipkoan commented Feb 25, 2019

This is an implementation of the [RFC] Server-side atlantis.yaml

@jjulien - can you comment as to what may be different or missing in this implementation from what is documented in the RFC?

@jjulien
Copy link
Contributor Author

jjulien commented Feb 25, 2019

@kipkoan I followed the RFC with the exception of two changes that I commented on in the RFC doc but were never officially included.

  1. Moved the new repo config options into their own file rather than including them in the server config.yaml. The new repo config is specified using the CLI option --repo-config. The file implements the repos and workflows keys using the schema defined in the RFC.
  2. Deprecated rather than removed --allow-repo-config, and implemented the repo restrictions as new functionality. The new functionality is triggered when the --allow-restricted-repo-config flag is used. The --allow-repo-config flag is now marked as deprecated (which prevents it from showing up in help as implemented by Cobra), and only one of either --allow-restricted-repo-config or --allow-repo-config is allowed to be used.

@TraGicCode
Copy link

TraGicCode commented Feb 27, 2019

Hey @jjulien @lkysow

While I haven’t looked at the code I read the RFC and really excited about this feature. This will address our concerns I’ve been having in regards to any pull request creator automatically having sudo “so to speak” on the Atlantis server which makes it hard to justify with the security team as “we just have to trust Users at our org will be not be malicious” in an enterprise environment but theoretically could take down production and/or expose sensitive secrets from terraforms remote state storage.

@jjulien jjulien force-pushed the iss_47_server_yaml branch 3 times, most recently from 540c07c to 0588a06 Compare February 28, 2019 20:51
@jjulien jjulien force-pushed the iss_47_server_yaml branch from 83d09ab to 8edd9bb Compare March 10, 2019 13:20
@jjulien
Copy link
Contributor Author

jjulien commented Mar 14, 2019

@lkysow This is ready to go now with the changes you mentioned in the RFC.

  • Removal of --allow-restricted-repo-config option, and making this the default behavior.
  • Deprecated --allow-repo-config

@lkysow
Copy link
Member

lkysow commented Mar 14, 2019

Thanks @jjulien! I'm going to pull this into my own branch and make some finishing touches.
Can you squash your commits into 1 please.

This enables atlantis.yaml in all repos, but by default restricts
certain sensitive keys from being used.

The keys apply_requirements, workflow, and workflows can only be
specified in an atlantis.yaml file if explicitly allowed by a
server side repo config.

The repo config file provides the ability to specify a default set of
workflows, and default values for apply_requirements and workflow to use
use on a per repo basis.  It also supports applying to a collection of
repos by using regex to match a repo name.

If more than one repo name matches, the values from last repo matched
are used.

This deprecates the --allow-repo-config option
@jjulien jjulien force-pushed the iss_47_server_yaml branch from 8e03fed to 9819e5f Compare March 14, 2019 20:16
@jjulien
Copy link
Contributor Author

jjulien commented Mar 14, 2019

@lkysow Commits have been squashed. Thanks!

@lkysow
Copy link
Member

lkysow commented Mar 14, 2019

Thanks! I'm going to merge something that might cause conflicts here but don't worry about fixing it, I will handle that.

@lkysow
Copy link
Member

lkysow commented Mar 19, 2019

Question: why did you add a new

FullNameWithHost:  "github.com/owner/repo"

field. Instead of adding a method that generates that data using existing fields:

// ID returns the atlantis ID for this repo.
// ID is in the form: {vcs hostname}/{repoFullName}.
func (r Repo) ID() string {
    return fmt.Sprintf("%s/%s", r.VCSHost.Hostname, r.FullName)
}

The only difference is that you're adding the port number which I don't think is necessary info.

@lkysow
Copy link
Member

lkysow commented Mar 25, 2019

Hi all subscribers,
If you're interested in trying out server-side config, I've created an alpha release off of @jjulien's original work and my refinement. You can get it here: https://github.com/runatlantis/atlantis/releases/tag/v0.7.0-alpha1 or from Docker: runatlantis/atlantis:v0.7.0-alpha.

• You can read docs about it here: https://deploy-preview-546--runatlantis.netlify.com/docs/repos-yaml-reference.html#overview
• If you've got feedback, either reply here or open up an issue. Thanks!

@lkysow
Copy link
Member

lkysow commented Mar 27, 2019

@jjulien I've refactored your changes and pushed everything to a release-0.7 branch because I don't want to merge into master and have the website update. If you're curious you can see the diff here: https://github.com/runatlantis/atlantis/compare/release-0.7?expand=1

Once 0.7 is ready I'll merge that branch into master.

I'm going to close this due to ☝️. Thanks for all your hard work 🙏! Should hopefully get a release out soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants