Skip to content

Trace sensitive attributes in custom dev builds#2478

Merged
carolynvs merged 3 commits intogetporter:mainfrom
carolynvs:trace-sensitive-attributes
Dec 6, 2022
Merged

Trace sensitive attributes in custom dev builds#2478
carolynvs merged 3 commits intogetporter:mainfrom
carolynvs:trace-sensitive-attributes

Conversation

@carolynvs
Copy link
Member

@carolynvs carolynvs commented Dec 2, 2022

What does this change

Instead of adding and removing println statements that write out sensitive data to assist with troubleshooting while writing code, only to have to remember to remove them before committing, I've added tracing.SetSensitiveAttributes.

In all normal builds created by go build, or our magefile targets, that function is a noop and does nothing. However if you build with go build -tags traceSensitiveAttributes, then that function will write out the passed attributes (which contain sensitive data) to the listening open telemetry collector (if any).

When porter runs with traceSensitiveAttributes set, a warning is printed to stderr to indicate the different behavior.

I hope this will give us a way to make it easy to debug in development while ensuring that we don't ever actually trace sensitive attributes in real builds.

What issue does it fix

I really needed this when I was refactoring parameter resolution, and in the past I know we have accidentally committed println statements that we shouldn't have. So this puts in a safer way to help us develop this area of the code.

Notes for the reviewer

I considered making this a configuration setting, but I thought this did a better job of making it real hard to someone to accidentally leak sensitive data. Most people don't do their own builds, let alone with artisanal build flags so if you end up with a build of Porter that traces sensitive data, it's clear they opted into it.

❓ I'm open to suggestions for the name of SetSensitiveAttributes. I mimicked the SetAttributes function but maybe we should stuff more info about how the function behaves into the name?

Checklist

  • Did you write tests?
  • Did you write documentation?
  • Did you change porter.yaml or a storage document record? Update the corresponding schema file.
  • If this is your first pull request, please add your name to the bottom of our Contributors list. Thank you for making Porter better! 🙇‍♀️

Reviewer Checklist

  • Comment with /azp run test-porter-release if a magefile or build script was modified
  • Comment with /azp run porter-integration if it's a non-trivial PR

@carolynvs carolynvs force-pushed the trace-sensitive-attributes branch from 75913b8 to 8f6d2e7 Compare December 2, 2022 17:34
Instead of adding and removing println statements that write out sensitive data to assist with troubleshooting while writing code, only to have to remember to remove them before committing, I've added tracing.SetSensitiveAttributes.

In all normal builds created by go build, or our magefile targets, that function is a noop and does nothing. However if you build with go build -tags traceSensitiveAttributes, then that function will write out the passed attributes (which contain sensitive data) to the listening open telemetry collector (if any).

When porter runs with traceSensitiveAttributes set, a warning is printed to stderr to indicate the different behavior.

I hope this will give us a way to make it easy to debug in development while ensuring that we don't ever actually trace sensitive attributes in real builds.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
@carolynvs carolynvs force-pushed the trace-sensitive-attributes branch from 8f6d2e7 to 9211900 Compare December 2, 2022 18:37
Support quickly tracing the json value of an object.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
Demonstrate how to use span.SetSensitiveAttributes by tracing the CNAB claim and credentials (both of which can contain sensitive data) when a bundle is run.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
@carolynvs carolynvs marked this pull request as ready for review December 2, 2022 20:13
Copy link
Contributor

@VinozzZ VinozzZ left a comment

Choose a reason for hiding this comment

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

lgtm!

@carolynvs carolynvs merged commit 89fb41d into getporter:main Dec 6, 2022
@carolynvs carolynvs deleted the trace-sensitive-attributes branch December 6, 2022 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants