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

Do not post results of plan and apply on git comments #938

Closed
nokernel opened this issue Feb 28, 2020 · 9 comments
Closed

Do not post results of plan and apply on git comments #938

nokernel opened this issue Feb 28, 2020 · 9 comments
Labels
feature New functionality/enhancement wont-do Unfortunately due to scope or project direction we won't build this in a reasonable amount of time

Comments

@nokernel
Copy link

Do not post results of plan and apply on git comments.

Since terraform can output secret on plan and apply outputs, keep the result of the plan / apply on the atlantis server.

Then post the final result success/fail to git comment with a link to atlantis server to get the detail of the output.

This way one user could shield the result with a authenticated proxy to secure the output of plan/apply and not post credentials as git comment.

@pratikmallya
Copy link
Contributor

pratikmallya commented Feb 29, 2020

You can do this using a custom workflow. e.g.

    workflows:
      workflow1:
        plan:
          steps:
          - run: terraform init -input=false -no-color -get-plugins=false > /dev/null
          - run: terraform plan -no-color > /atlantis/data/plan/blah
        apply:
          steps:
          - run: terraform apply -auto-approve > /atlantis/data/apply/blah 

@nokernel
Copy link
Author

nokernel commented Mar 1, 2020

You can do this using a custom workflow. e.g.

    workflows:
      workflow1:
        plan:
          steps:
          - run: terraform init -input=false -no-color -get-plugins=false > /dev/null
          - run: terraform plan -no-color > /atlantis/data/plan/blah
        apply:
          steps:
          - run: terraform apply -auto-approve > /atlantis/data/apply/blah 

Well I I am corrected ;)

Now I could point some plain web-server in front of those files.

What would be great is to have this integrated as a feature, to "officially" keep those files local and host them on the UI with a posted link on the git comment.

But good thinking there.

The flow I would see is :

  1. terraform command's output saved on atlantis
  2. atlantis post result success/fail ...as git comment with link to atlantis UI containing the details.

@wlonkly
Copy link

wlonkly commented Mar 1, 2020

As a counterargument,

to "officially" keep those files local and host them on the UI

I depend on Atlantis being (practically) stateless. If the container running Atlantis gets rescheduled, worst case I lose track of locks in a failsafe manner. If Atlantis needs to store output, it needs to be in a datastore, not the local filesystem (but I'd hate for Atlantis to require a datastore).

@nokernel
Copy link
Author

nokernel commented Mar 1, 2020

but I'd hate for Atlantis to require a datastore

Good point.

How would we handle secrets in terraform output?

Keeping in mind that the output of plan and apply can be useful for debugging and to validate changes before applying.

Not having a history of those output is not a great loss I think. When the PR is merged I don't see great value in them. So maybe losing them is not that critical?

@wlonkly
Copy link

wlonkly commented Mar 1, 2020

How would we handle secrets in terraform output?

Heh, I just had this conversation in a different issue, but Terraform and not Atlantis should handle the secrets (through the sensitive parameter in an output, or in the provider), because Atlantis supports any Terraform provider and can't be expected to know what's going on "inside" them.

If you're getting a sensitive value in a Terraform output, mark it as sensitive, and if you're getting a sensitive value displayed in the contents of a plan, it's a bug in the associated Terraform provider.

When the PR is merged I don't see great value in them. So maybe losing them is not that critical?

Absolutely critical for analysis if the apply causes an incident, especially in cases where the apply only half-applied because it got an error from the provider's API.

I mean, that's the a big part of the value of Atlantis for me, that it stores the result of the change in the same place as the record of the code that made the change.

@nokernel
Copy link
Author

nokernel commented Mar 1, 2020

@wlonkly I understand your point, but I still disagree :), but hey I think Atlantis should support both ways.

The issue at terraform to hide secrets in variable is open since 2017 and maybe earliser and we are not even talking about secrets in locals here.

Even then this wont fix issue when, you pull a secret from vault and pass it to another module then you see the secret, or if you put that secret in a template, you also see the secret in plain.

I dont see this getting fixed any time soon, hence this request. I am not proposing to hide secret of terraform by trying to regex them out like tfmask or tfhelper.

I suggest that atlantis have a parameter to say "keep terraform output local" and post result + status on git comment with link to full terraform output.

I think that this is a valid use-case, and a DB can even be put in placce to safeguard the terraform outputs if you want to keep them for history. Obviously I see this as optional, one could ask atlantis to keep those files locally ( if on k8s just put that on persistent volume ), or in a DB postgres, mariadb, or alike...

They are very easy to setup and could safeguard the outputs.

@lkysow lkysow added the feature New functionality/enhancement label Mar 2, 2020
@lkysow
Copy link
Member

lkysow commented Mar 2, 2020

Hi Folks,
Overall I think this is not something that Atlantis should support. Posting all interaction to the PR as comments is the core workflow of Atlantis. If you're not comfortable with the security issues that results in, I'd suggest using Terraform Cloud.

To be clear, I think this is a totally valid feature request and I understand what's led to this request however at some point we need to pick a direction and draw a line for what a tool's purpose and functionality is so that it doesn't get overly complicated and feature work can be focused around a core workflow.

@mwarkentin
Copy link
Contributor

I think you could also use something like https://github.com/cloudposse/tfmask

@lkysow
Copy link
Member

lkysow commented Mar 27, 2020

Closing due to #938 (comment)

@lkysow lkysow closed this as completed Mar 27, 2020
@lkysow lkysow added the wont-do Unfortunately due to scope or project direction we won't build this in a reasonable amount of time label Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality/enhancement wont-do Unfortunately due to scope or project direction we won't build this in a reasonable amount of time
Projects
None yet
Development

No branches or pull requests

5 participants