Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data source based on OSS backend doesn't support nested parameters .assume_role["role_arn"]: an object is required. #29305

Closed
hayorov opened this issue Aug 5, 2021 · 9 comments
Labels
backend/oss bug new new issue not yet triaged

Comments

@hayorov
Copy link
Contributor

hayorov commented Aug 5, 2021

Terraform Version

Terraform v1.0.4
on linux_amd64

Terraform Configuration Files

data "terraform_remote_state" "level1" {
    backend   = "oss"
    config    = {
      bucket                = "XXX-XXX-XX"
      prefix                = "level1/networking"
      key                   = "terraform.tfstate"
      acl                   = "private"
      encrypt               = true
      region                = "ap-southeast-1"
      access_key = jsondecode(chomp(file(".provider_access"))).access_key
      secret_key = jsondecode(chomp(file(".provider_access"))).secret_key
      assume_role  = {
        role_arn = "acs:ram::XXX:role/resourcedirectoryaccountaccessrole"
      }
    }
}

Debug Output

The given configuration is not valid for backend "oss":
.assume_role["role_arn"]: an object is required.

Expected Behavior

Data source works as described in documentation with parameters defined in nested configuration block.

Actual Behavior

A Validation error raises on any parameter used nested assume_role block. For example .assume_role["role_arn"]: an object is required.

Steps to Reproduce

  1. create file remote.tf with context
  data "terraform_remote_state" "level1" {
      backend   = "oss"
      config    = {
        bucket                = "XXX-XXX-XX"
        prefix                = "level1/networking"
        key                   = "terraform.tfstate"
        acl                   = "private"
        encrypt               = true
        region                = "ap-southeast-1"
        access_key = "XXX"
        secret_key = "YYY"
        assume_role  = {
          role_arn = "acs:ram::XXX:role/resourcedirectoryaccountaccessrole"
        }
      }
  }
  1. run terraform init

Additional Context

Official documentation https://www.terraform.io/docs/language/settings/backends/oss.html#data-source-configuration

Remote backend (OSS) works fine.
Meanwhile, data source has its own schema mapping/validation, and based on my research the problem happens in the data_source_state during config to schema mapping.

# internal/builtin/providers/terraform/data_source_state.go
	configVal, err := schema.CoerceValue(config)  // <- here 
	if err != nil {
		diags = diags.Append(tfdiags.AttributeValue(
			tfdiags.Error,
			"Invalid backend configuration",
			fmt.Sprintf("The given configuration is not valid for backend %q: %s.", backendType,
				tfdiags.FormatError(err)),
			cty.Path(nil).GetAttr("config"),
		))
		return nil, cty.NilVal, diags
	}

# internal/configs/configschema/coerce_value.go
func (b *Block) coerceValue(in cty.Value, path cty.Path) (cty.Value, error) {
	switch {
	case in.IsNull():
		return cty.NullVal(b.ImpliedType()), nil
	case !in.IsKnown():
		return cty.UnknownVal(b.ImpliedType()), nil
	}

	ty := in.Type()
	if !ty.IsObjectType() {
		return cty.UnknownVal(b.ImpliedType()), path.NewErrorf("an object is required"). // <- here
	}

Also, there is no other backend that declares nested block under configuration. A good example is the s3 backend, which has similar functionality with assume_role and has a flat parameters design. Ref https://www.terraform.io/docs/language/settings/backends/s3.html#assume-role-configuration

assume_role_duration_seconds, assume_role_policy, assume_role_transitive_tag_keys

So it seems that this feature has never worked since the initial implementation. Based on other backend designs, the validation/schema mapping doesn't support nested blocks under the configuration section of the backrnd.

References

#23755 closed as "This issue doesn't seem to be following one of the standard-issue templates..."

@hayorov
Copy link
Contributor Author

hayorov commented Aug 6, 2021

Thx, @jbardin. @xiaozhu36 could you please take a look?

@hayorov
Copy link
Contributor Author

hayorov commented Aug 10, 2021

Hello, any news here?

@hayorov
Copy link
Contributor Author

hayorov commented Oct 11, 2021

@jbardin it seems that the component maintainer is not available. I've tried all available channels, including Alibaba corporate email and internal Alibaba representatives. What can be next? Any ideas on how to contribute? I'm extremely open to any options.

ping @xiaozhu36
syn @xiaozhu36

@hayorov
Copy link
Contributor Author

hayorov commented Oct 18, 2021

Pr is updated and review is pending #29307

@xiaozhu36
Copy link
Contributor

HI @hayorov There is an incompatible update and please have a check.

@hayorov
Copy link
Contributor Author

hayorov commented Oct 22, 2021

HI @hayorov There is an incompatible update and please have a check.

Hello, it's now fully backward compatible and I've resolved all your comments, please take a look.

@apparentlymart
Copy link
Contributor

Hi all,

I'm ultimately happy to let the maintainers of the backend decide what's the best thing to do here, but I did want to note that the error message here seems correct, despite being confusingly-worded, because assume_role is defined in the provider schema as being represented as a set of zero or one elements:

return &schema.Schema{
Type: schema.TypeSet,
Optional: true,
MaxItems: 1,
Elem: &schema.Resource{

Because of this, I think the correct syntax to write a value for that argument in this data source context where we're providing a direct value conforming to the schema, rather than a configuration body that decodes into the schema, would be like this:

data "terraform_remote_state" "level1" {
  backend   = "oss"
  config = {
    # (other arguments as in the original example)
    assume_role  = [
      {
        role_arn           = "acs:ram::XXX:role/resourcedirectoryaccountaccessrole"
        session_name       = null
        policy             = null
        session_expiration = null
      },
    ]
  }
}

What I've written above is an expression to produce the same value that the configuration decoder would normally produce in order to conform to the schema. Because config is an attribute rather than a block it can't contain nested blocks in the way a configuration body normally could, and so to make it work we need to manually describe what the configuration decoder would've produced.

It is unfortunate that the terraform_remote_state data source works differently here, but it's intended to make it easier to generate a totally-dynamic value for config, whereas using block syntax here would mean the need to use dynamic blocks and similar in more complicated cases.

It might still be preferable to flatten the configuration in order to retain more consistency between this situation and the backend "oss" block structure, but I hope the above is helpful for those who want to make it work with the backend as it's designed today.

@hayorov
Copy link
Contributor Author

hayorov commented Oct 30, 2021

Many thanks @apparentlymart it's a good explanation and tbh I guessed during the implementation of #29307.

It seems that the same problem/question was asked before in #23755 and structures [{}, ] doesn't look organic (IMO).

I've supported a flattened variant with backward compatibility for the existing format and got maintainer approval here
#23755

I'm sure that generalization across backends is a positive thing and will make the OSS backend more homogenous.
I Hope, you (the Hashi team) could assist with merging and I'm happy to close this issue.

Once again, many thanks @apparentlymart @jbardin and @xiaozhu36.

@jbardin jbardin closed this as completed Nov 1, 2021
@github-actions
Copy link

github-actions bot commented Dec 2, 2021

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backend/oss bug new new issue not yet triaged
Projects
None yet
Development

No branches or pull requests

4 participants