terraform output should adhere to Terraform Cloud authorization
#31507
Conversation
Normally, `terraform output` refreshes and reads the entire state in the command package before pulling output values out of it. This doesn't give Terraform Cloud the opportunity to apply the read state outputs org permission and instead applies the read state versions permission. I decided to expand the state manager interface to provide a separate GetRootOutputValues function in order to give the cloud backend a more nuanced opportunity to fetch just the outputs. This required moving state Refresh/Read code that was previously in the command into the shared backend state as well as the filesystem state packages.
brandonc
force-pushed
the
brandonc/output_cloud_reads
branch
from
fdbdf74
to
e1fa690
Compare
internal/cloud/state.go
Outdated
| @@ -70,6 +73,22 @@ func (s *State) WriteState(state *states.State) error { | |||
| return s.delegate.WriteState(state) | |||
| } | |||
|
|
|||
| func (s *State) fallbackReadOutputsFromFullState() (map[string]*states.OutputValue, error) { | |||
| if err := s.RefreshState(); err != nil { | |||
| if strings.HasSuffix(err.Error(), "failed to retrieve state: forbidden") { | |||
There was a problem hiding this comment.
this is probably resource not found in production 🤔
|
@jbardin Thanks, James! When you say existing patterns do you mean the Persistent interface? Is there anything I can do to improve this now, such as move my new method to the state Reader interface? |
|
I wouldn't worry about it. We need to refactor the state/backend abstractions in general, so let's just leave it for now. The individual composed interfaces were never really useful, and make reading which interface does what or deciding which to use more difficult, but I think breaking from the existing pattern for a new single method would only be more confusing. |
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Normally,
terraform outputrefreshes and reads the entire state in the command package before pulling output values out of it. This doesn't give Terraform Cloud the opportunity to apply the "read state outputs" org permission and instead requires the elevated "read state versions" permission.I decided to expand the state manager interface to provide a separate GetRootOutputValues function in order to give the cloud backend a more nuanced opportunity to fetch just the outputs. This required moving state Refresh/Read code that was previously in the command into the shared backend state as well as the filesystem state packages.
Notes
Since terraform is now consuming outputs from the API, the API supports a new "detailed-type" output attribute (depending on feature flags) which contains the terraform type encoded as the terraform-json tuple. The type and value are then used to recreate the output type with a full cty Value that should give identical results to the local state version.
Testing Steps
terraform output- you should see the same results as before.Example Config
terraform { backend "remote" { hostname = "app.staging.terraform.io" token = "bcroft" organization = "bcroft"}
}
resource "null_resource" "test" {
triggers = {
hello = "wat11"
}
}
resource "random_id" "random" {
keepers = {
uuid = uuid()
}
byte_length = 8
}
output "an-object" {
value = {
nesting1: {
nesting2: {
nesting3: null_resource.test.id
}
}
}
}
output "random" {
value = random_id.random.hex
description = "a random hex output"
}
output "a-true-bool" {
value = true
description = "The true of the main server instance."
}
output "escapes" {
value = "line 1\nline 2\n\\\\\n"
description = "The true of the main server instance."
}
output "a-false-bool" {
value = false
description = "The false of the main server instance."
}
output "a-string" {
value = "example string"
description = "The private string of the main server instance."
}
output "an-int" {
value = 1001
description = "The private integer of the main server instance."
}
output "a-decimal" {
value = 1000.1
description = "The private decimal of the main server instance."
}
output "a-list" {
value = tolist(["example", 1001, 1000.1])
description = "The list"
}
output "a-map" {
value = tomap({ "example" : 1001, "bar" : 1000.1 })
description = "The map"
}
output "a-tuple" {
value = ["1", "example"]
description = "The tuple"
}
output "a-long-string" {
value = "The private integer of the main server instance is where you want to go when you have the most fun in every Terraform instance you can see in the world that you live in except for dogs because they don't run servers in the same place that humans do."
description = "A long string is good for you."
}
output "a-sensitive-value" {
value = "hopefully you cannot see me"
description = "The sensitive string of the main server instance."
sensitive = true
}
Example output
When reading state outputs from earlier versions of terraform (that do not send the detailed type information) terraform will fall back to reading the entire state file. If that fails, a friendly error is shown:
TODO: validate that users that cannot read the full state see a 403 forbidden error instead of a 404 not found error