feat: allow masking output on comments

[GMartinez-Sisti]

what

Part of #163 (comment).

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.

Example (added to the docs):

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

Note that the changes related to mocks were automatically generated with make go-generate.

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 #163.

[jamengual]

did you test tfmask? or any other tool?

[GMartinez-Sisti]

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.

[jamengual]

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

[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.

[GMartinez-Sisti]

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.

[X-Guardian]

Hi @GMartinez-Sisti, are you able to look at the suggestions from @anryko. It would be great to get this merged.

[GMartinez-Sisti]

I've been thinking about the suggested API, the suggested output accepts either:

• a string
• a list of strings and a list of maps

I think this is not ideal and might create some confusion, we can support multiple types but only one at a time and act accordingly. This is my suggestion:

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
          regex_expression: "((?i)secret:\\s\")[^\"]*"

• output accepts either string or map[string]string
• regex_expression has a default value and we can optionally define it to set a different value

WDYT @anryko and @X-Guardian ?

[anryko]

The api I suggested would provide an option to apply a sequence of simple regexps one after another. It would make your feature more powerful. I understand the added implementation complexity you are referring to and believe that this would be a bit easier to implement on top of the changes done for this feature, which "loosens" the config unmarshaling.

[GMartinez-Sisti]

The api I suggested would provide an option to apply a sequence of simple regexps one after another. It would make your feature more powerful. I understand the added implementation complexity you are referring to and believe that this would be a bit easier to implement on top of the changes done for this feature, which "loosens" the config unmarshaling.

I see it, while being more verbose it will be more flexible indeed. I'll wait for #5024 to be merged then so I can leverage the new map[string]map[string]interface{}.

[X-Guardian]

Hi @GMartinez-Sisti, #5024 is now merged. Can you resolve the conflicts on this?

[GMartinez-Sisti]

Hi @GMartinez-Sisti, #5024 is now merged. Can you resolve the conflicts on this?

I've fixed the conflicts, but haven't updated the logic to match the suggestions.

[github-actions]

This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month.

[GMartinez-Sisti]

Will still work on this! ☺️