Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

Output variable values used and obfuscate sensitive values #3138

Merged
merged 22 commits into from
Mar 30, 2022

Conversation

krantzinator
Copy link
Contributor

@krantzinator krantzinator commented Mar 24, 2022

This PR adds support for obfuscating variables set to sensitive in the waypoint.hcl, and also supports displaying the final variable values in CLI output and logs, as well as the UI (though this part is only supported and not yet implemented).

This will allow users to see the used variable values upon the completion of a job, as well as enable us to add support for actually looking up which values were used on a given build/deploy/release.
This also enables future improvements such as a "noop" dry-run that returns the values that would be used if an operation was completed.

sensitive values are called sensitive to match what Terraform and Packer call them, to give consistency among our HashiCorp products. Unique to Waypoint though is the obfuscation of the values by using SHA256 encoding. This is not meant to be a security mechanism -- the values are stored on the server, which must be secured by the user -- but it does serve as a way to both obfuscate the values in logging and other outputs, while still enabling a user to verify that the value they expected the job to run with was the final value used.

The final values are stored as Variable_Refs so as to separate the values set on the server (when a user saves a value in the UI) and the final set of values used. This also enables an easier format for easily translating to logs and outputs.

The Sensitive item is added to the Variable message to support further communication between the UI and the variables set in the waypoint.hcl as part of future improvements.

To see it in action, you can use the below waypoint.hcl on our waypoint-examples/docker/go project:

project = "example-go"

app "example-go" {
  labels = {
    "service" = "example-go",
    "env"     = "dev"
  }

  build {
    use "pack" {}

    registry {
      use "docker" {
        image = "rae/go"
        tag   = var.tag
        local = var.local
      }
    }
  }

  deploy {
    use "docker" {}
  }
}

variable "tag" {
  type = string
  sensitive = true
  default = "hello"
}

variable "local" {
  default = true
}

To see how complex HCL types come through, replace the variable definition and usage of tag with:

...
        tag   = var.tag[0]
...
variable "tag" {
        type       = list(string)
        default   = ["hello", "othertag", "moretag", "whatifi", "havelotsof", "tagsand things"]
}

A couple of screenshots showing some example outputs:
Screen Shot 2022-03-23 at 9 51 38 PM

An example of the line breaks I was going for; without the manual line break intervention, the use of variable values longer than the terminal width severely messed up the column title widths:
Screen Shot 2022-03-23 at 9 53 05 PM

@krantzinator krantzinator added enhancement New feature or request core cli pr/no-changelog No automatic changelog entry required for this pull request labels Mar 24, 2022
@krantzinator krantzinator added this to the 0.8.0 milestone Mar 24, 2022
@krantzinator krantzinator requested a review from a team March 24, 2022 02:07
@krantzinator krantzinator removed the pr/no-changelog No automatic changelog entry required for this pull request label Mar 24, 2022
@briancain
Copy link
Member

Oops, I hit submit too fast! I also ran into a weird thing where this string input var was right aligned? Very strange, not sure what's going on there 🤔

» Variables used:
     VARIABLE    |         VALUE         |  TYPE  | SOURCE
-----------------+-----------------------+--------+----------
  image          | localhost:5000/tetris | string | cli
  namespace      |                       | string | default
  registry_local | false                 | bool   | cli
  release_port   |                  3000 | string | default
  tag            | latest                | string | default

The input var is defined as:

variable "release_port" {
  default     = "3000"
  type        = string
  description = "Port to open for the releaser."
}

@briancain
Copy link
Member

This looks awesome @krantzinator !! Excited for this feature to land. I haven't had a chance to review the rest, mostly coming at it from the UX side. But looking pretty good from that direction 👍🏻

for i := range val {
// line break every 45 characters
if i%46 == 0 && i != 0 {
val = val[:i] + "\n" + val[i:]
Copy link
Contributor

Choose a reason for hiding this comment

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

As is, the first chunk would be 46 chars because of indexing.

Suggested change
val = val[:i] + "\n" + val[i:]
val = val[:i-1] + "\n" + val[i:]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm just going to make this i%45; the indexing 😵 got me but that's what I want -- up to the first 45 chars inclusive (index 0-44), and then the next set exclusive ([45:].

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I tested this and if you do i%45 it doesn't crop right! The first chunk ends up as 45 chars but every subsequent chunk is only 44. @krantzinator

internal/cli/deployment_create.go Outdated Show resolved Hide resolved
internal/cli/release_create.go Outdated Show resolved Hide resolved
internal/cli/up.go Outdated Show resolved Hide resolved
internal/config/variables/variables.go Outdated Show resolved Hide resolved
internal/config/variables/variables.go Outdated Show resolved Hide resolved
website/content/docs/waypoint-hcl/variables/input.mdx Outdated Show resolved Hide resolved
Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

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

I didn't do a full line by line review because I think there is enough in my feedback.

The core logic here feels really solid. I have some important feedback on the structure of the data and where it lives. But the actual core encoding of the values and storage generally is great.

pkg/server/proto/server.proto Outdated Show resolved Hide resolved
pkg/server/proto/server.proto Outdated Show resolved Hide resolved
pkg/server/proto/server.proto Outdated Show resolved Hide resolved
pkg/server/proto/server.proto Outdated Show resolved Hide resolved
pkg/server/proto/server.proto Outdated Show resolved Hide resolved
internal/runner/operation.go Outdated Show resolved Hide resolved
internal/runner/operation.go Show resolved Hide resolved
@xiaolin-ninja
Copy link
Contributor

xiaolin-ninja commented Mar 25, 2022

Just in case this comment got lost in the rebasing, for the manual line break logic, just changing it to i%45 like:

if i%45 == 0 && i != 0 {
  val = val[:i] + "\n" + val[i:]
  ...
}

won't fix it, because the first chunk will be alright at 45 chars, but subsequent chunks will be 44 chars.

But my original proposed solution also isn't right either, because it'll drop a char in between each chunk. This should work:

	if len(val) > 45 {
		for i := 45; i < len(val); i += 46 {
			val = val[:i] + "\n" + val[i:]
		}
	}

@krantzinator krantzinator merged commit 61e3f05 into main Mar 30, 2022
@krantzinator krantzinator deleted the f-sensitive-vars branch March 30, 2022 14:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
4 participants