Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

Run atlantis container as atlantis user instead of root #11197

Merged
merged 1 commit into from
Feb 6, 2019

Conversation

chrisob
Copy link
Contributor

@chrisob chrisob commented Feb 6, 2019

Signed-off-by: Chris O'Brien [email protected]
@jkodroff @callmeradical @jeff-knurek @lkysow

What this PR does / why we need it:

Since atlantis v0.4.12 (specifically this PR), any custom gitconfig options are ignored, as the container/pod is running as root instead of atlantis.

Which issue this PR fixes

Run the pod as UID 100, AKA the atlantis user.
Remove the atlantis command from the pod spec so as not to bypass docker-entrypoint.sh, which gosus to the atlantis user.

Special notes for your reviewer:

Tested and working, my custom gitconfig specified in values.yaml works again.

Checklist

  • DCO signed
  • Chart Version bumped
  • Variables are documented in the README.md

@helm-bot helm-bot added Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 6, 2019
@lkysow
Copy link
Contributor

lkysow commented Feb 6, 2019

Hi Chris,
The Atlantis docker image has a provision to run as the atlantis user: https://github.com/runatlantis/atlantis/blob/master/docker-entrypoint.sh#L43

What's happening though is that the helm chart is setting

          command: ["atlantis"]

https://github.com/helm/charts/blob/master/stable/atlantis/templates/statefulset.yaml#L64

Which overrides the Docker Entrypoint (https://kubernetes.io/docs/tasks/inject-data-application/define-command-argument-container/#notes).

Instead, I think we should change:

          command: ["atlantis"]
          args:
            - server

to

          args:
            - server

Which should allow the default entrypoint to work.

Would you mind seeing if that fixes things?

Copy link
Contributor

@lkysow lkysow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See ☝️

@helm-bot helm-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 6, 2019
@chrisob
Copy link
Contributor Author

chrisob commented Feb 6, 2019

@lkysow Yep, good call, I can confirm removing the atlantis command stops bypassing docker-entrypoint.sh, and atlantis server runs as the atlantis user 😁

@k8s-ci-robot
Copy link
Contributor

@chrisob: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

@lkysow Yep, good call, I can confirm removing the atlantis command stops bypassing docker-entrypoint.sh, and atlantis server runs as the atlantis user 😁

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@lkysow
Copy link
Contributor

lkysow commented Feb 6, 2019

/ok-to-test

@lkysow
Copy link
Contributor

lkysow commented Feb 6, 2019

Nice! Please squash your commits.

Signed-off-by: Chris O'Brien <[email protected]>

Bump chart version

Signed-off-by: Chris O'Brien <[email protected]>

Revert "Run atlantis container as atlantis user instead of root"

This reverts commit 8938f3f

Signed-off-by: Chris O'Brien <[email protected]>

Remove explicit atlantis command so as not to bypass docker entrypoint

Signed-off-by: Chris O'Brien <[email protected]>
@helm-bot helm-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 6, 2019
@chrisob
Copy link
Contributor Author

chrisob commented Feb 6, 2019

@lkysow done

Copy link
Contributor

@lkysow lkysow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

@lkysow
Copy link
Contributor

lkysow commented Feb 6, 2019

/lgtm

1 similar comment
@lkysow
Copy link
Contributor

lkysow commented Feb 6, 2019

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrisob, lkysow

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. labels Feb 6, 2019
@k8s-ci-robot k8s-ci-robot merged commit 71e20cf into helm:master Feb 6, 2019
@lkysow
Copy link
Contributor

lkysow commented Feb 6, 2019

Thanks @chrisob! 🎉

@chrisob chrisob deleted the atlantis_runasuser branch February 6, 2019 23:06
@chrisob
Copy link
Contributor Author

chrisob commented Feb 6, 2019

@lkysow thank you for such a great tool in the first place!

@lkysow lkysow mentioned this pull request Feb 7, 2019
3 tasks
towolf pushed a commit to towolf/charts that referenced this pull request Feb 7, 2019
Signed-off-by: Chris O'Brien <[email protected]>

Bump chart version

Signed-off-by: Chris O'Brien <[email protected]>

Revert "Run atlantis container as atlantis user instead of root"

This reverts commit 8938f3f

Signed-off-by: Chris O'Brien <[email protected]>

Remove explicit atlantis command so as not to bypass docker entrypoint

Signed-off-by: Chris O'Brien <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). lgtm Indicates that a PR is ready to be merged. ok-to-test size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants