atlantis icon indicating copy to clipboard operation
atlantis copied to clipboard

feat: allow masking output on comments

Open GMartinez-Sisti opened this issue 1 year ago • 20 comments
trafficstars

what

Part of https://github.com/runatlantis/atlantis/issues/163#issuecomment-1986854570.

why

I have the requirements to mask some values that are passed to the comments posted by Atlantis, building up on strip_refreshing I added two new output configurations that will allow this via a regex configured on the step. There is an assumption that users that shouldn't see secrets/sensitive values won't have access to the URL jobs, where the plan outputs are shown untouched.

The output key can now contain a string, []stringor[]any`, this was we ensure compatibility while adding new possibilities to it.

Example (added to the docs):

workflows:
  terragrunt:
    plan:
      steps:
      - run:
          command: terragrunt plan -input=false -out=$PLANFILE
          output:
            - strip_refreshing
            # Filters text matching 'mySecret: "aaa"' -> 'mySecret: "<redacted>"'
            - filter_regex: "((?i)secret:\\s\")[^\"]*"

Note that the changes related to mocks were made manually since make go-generate is currently broken (https://github.com/runatlantis/atlantis/issues/4664).

tests

  • Running all the tests locally and adding coverage for the new feature
  • Build and deployed this version with the new config from feature and tested that atlantis plan provides the desired masked output on GitHub 😄

references

Possibly solves https://github.com/runatlantis/atlantis/issues/163.

GMartinez-Sisti avatar Mar 10 '24 18:03 GMartinez-Sisti

did you test tfmask? or any other tool?

jamengual avatar Mar 10 '24 18:03 jamengual

did you test tfmask? or any other tool?

I did, also terrahelp and even plain sed. The problem is that we are sending the output straight to the $planfile, so we can’t act on it. I even tried to change the $showfile, and while that works, Atlantis doesn’t use it for the comment.

GMartinez-Sisti avatar Mar 10 '24 19:03 GMartinez-Sisti

I see ok, it make sense on doing the pre-processing

jamengual avatar Mar 10 '24 19:03 jamengual

I like the feature and find it very useful. However, IMHO, the API could be better. To maintain backward compatibility and allow for extending this API I would suggest the following implementation:

workflows:
  terragrunt:
    plan:
      steps:
      - run:
          command: terragrunt plan -input=false -out=$PLANFILE
          output: strip_refreshing|show|hide
      - run:
          command: terragrunt plan -input=false -out=$PLANFILE
          output:
            - show
            - strip_refreshing
            - filter_regex: "((?i)secret:\\s\")[^\"]*"

This would allow us to support previous string values and in case of the list[string], apply multiple filters sequentially, as well as add more filters in the future without breaking the API. The filter_regex is a better name because it matches <action> | <action>_<topic> naming pattern of already supported strip_refreshing|show|hide output actions.

anryko avatar May 28 '24 14:05 anryko

I like the feature and find it very useful. However, IMHO, the API could be better. To maintain backward compatibility and allow for extending this API I would suggest the following implementation:

workflows:
  terragrunt:
    plan:
      steps:
      - run:
          command: terragrunt plan -input=false -out=$PLANFILE
          output: strip_refreshing|show|hide
      - run:
          command: terragrunt plan -input=false -out=$PLANFILE
          output:
            - show
            - strip_refreshing
            - filter_regex: "((?i)secret:\\s\")[^\"]*"

This would allow us to support previous string values and in case of the list[string], apply multiple filters sequentially, as well as add more filters in the future without breaking the API. The filter_regex is a better name because it matches <action> | <action>_<topic> naming pattern of already supported strip_refreshing|show|hide output actions.

Hi, thanks for the feedback 😃

I've been using this to support terraform for 100+ environments on the three major clouds with zero issues so far. I adjusted the regex to "((?i)secret.*:\\s\")[^\"]*" so it includes pretty much all fields with secret on the name.

I have to rebase this soon, I'll take a stab at making it work the way you suggested and see how it behaves.

GMartinez-Sisti avatar May 28 '24 14:05 GMartinez-Sisti