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: Remove flag for allow repo config #3911

Merged
merged 11 commits into from
Dec 12, 2023

Conversation

lukemassa
Copy link
Contributor

@lukemassa lukemassa commented Nov 2, 2023

what

Remove flag --allow-repo-config

why

This has been deprecated since 1fcdaba.

That commit changed the semantics of --allow-repo-config, which is important to clarify for the flag's removal. Before the deprecation, --allow-repo-config was the only way to allow or disallow reading the atlantis.yaml in repos. Afterwards, however, the repo level atlantis.yaml is always read, and the new role of --allow-repo-config is to determine whether we should accept all values from the repo level yaml, or only particular ones, as specified in the new repo config.

This is why the default of --allow-repo-config is false. Removing this as an option means it will be impossible to specify that all values from the repo level yaml should be trusted, rather the repo config is necessary to allow overrides to important settings like apply_requirements.

I opted to keep AllowRepoConfig in GlobalCfgArgs, renaming it to AllowAllRepoSettings, since it is convenient for testing. It however is not set in server.go so is false in any production run.

I also removed two deprecated functions from global_cfg.go, which between them were only called once, a call site that was easy to translate to the supported NewGlobalCfgFromArgs.

tests

Running unit tests to confirm existing behavior. Actual flag was removed:

Error: unknown flag: --allow-repo-config

references

@lukemassa lukemassa changed the title Remove flag for allow repo config feat: Remove flag for allow repo config Nov 2, 2023
@lukemassa lukemassa marked this pull request as ready for review November 4, 2023 20:22
@lukemassa lukemassa requested a review from a team as a code owner November 4, 2023 20:22
@github-actions github-actions bot added the go Pull requests that update Go code label Nov 4, 2023
@jamengual jamengual added refactoring Code refactoring that doesn't add additional functionality waiting-on-review Waiting for a review from a maintainer labels Nov 15, 2023
@jamengual
Copy link
Contributor

@lukemassa, could you fix this conflict?

@lukemassa
Copy link
Contributor Author

@jamengual fixed!

GenPage
GenPage previously approved these changes Dec 12, 2023
@GenPage GenPage removed the waiting-on-review Waiting for a review from a maintainer label Dec 12, 2023
@GenPage GenPage enabled auto-merge (squash) December 12, 2023 01:26
GenPage
GenPage previously approved these changes Dec 12, 2023
@GenPage
Copy link
Member

GenPage commented Dec 12, 2023

@lukemassa test are failing with server/controllers/events/events_controller_e2e_test.go:1328:6: preWorkflowHooks declared and not used

https://github.com/runatlantis/atlantis/actions/runs/7176026475/job/19540268725?pr=3911#step:4:125

It looks like it happened after merging the main in (couldn't rebase). I'm checking the main branch commits

@GenPage
Copy link
Member

GenPage commented Dec 12, 2023

@lukemassa
Copy link
Contributor Author

Looking now

auto-merge was automatically disabled December 12, 2023 03:07

Head branch was pushed to by a user without write access

@GenPage GenPage added this to the v0.27.0 milestone Dec 12, 2023
@GenPage GenPage enabled auto-merge (squash) December 12, 2023 03:15
@GenPage GenPage added breaking-change feature New functionality/enhancement labels Dec 12, 2023
@GenPage GenPage merged commit d643c52 into runatlantis:main Dec 12, 2023
30 of 37 checks passed
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
* Remove option to allow repo

* Fix internal test

* Fix fmt

* Fmt

* Fix rebase

---------

Co-authored-by: PePe Amengual <[email protected]>
Co-authored-by: Dylan Page <[email protected]>
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
* Remove option to allow repo

* Fix internal test

* Fix fmt

* Fmt

* Fix rebase

---------

Co-authored-by: PePe Amengual <[email protected]>
Co-authored-by: Dylan Page <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change feature New functionality/enhancement go Pull requests that update Go code refactoring Code refactoring that doesn't add additional functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants