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

Optional/Computed Sets with Terraform Plugin Framework #28638

Closed
jar-b opened this issue Jan 3, 2023 · 3 comments · Fixed by #28643
Closed

Optional/Computed Sets with Terraform Plugin Framework #28638

jar-b opened this issue Jan 3, 2023 · 3 comments · Fixed by #28643
Labels
service/auditmanager Issues and PRs that pertain to the auditmanager service.
Milestone

Comments

@jar-b
Copy link
Member

jar-b commented Jan 3, 2023

Description

This issue documents research of optional/computed set attribute configurations with terraform-plugin-framework and protocol version 5 (Terraform AWS Provider). This includes various schema designs and alternatives for storing partial results to state.

Background

The AWS AuditManager Assessment resource includes a “Roles” attribute that is a set of objects containing AWS IAM role information. At least one element is required on Create and Update. The resulting response may contain additional elements, as an IAM role may have access to all assessments by default. This effectively makes the attribute both required and computed, but since this combination is not permitted an optional/computed set attribute was attempted.

Recommendation

Based on the research below, I’d propose moving forward with Multiple Set Attributes. This pattern still has the caveat of the Other Alternatives (no drift detection for roles not tracked in configuration), but with the benefit of those roles being visible in state via the roles_all attribute.

See this issue comment recommending a similar approach for a computed/optional map attribute.

Attempted Schema Designs

Because the AWS provider is limited to protocol version 5, nested attributes are not yet an option. Schema designs with Blocks and Set attributes are documented in detail below. Account numbers and role names have been redacted from error messages, but “redacted-admin” represents an IAM role that by default has permissions to all assessments, but isn’t included in the terraform configuration.

Block with Nested Optional/Computed Attributes

Schema:

			"roles": schema.SetNestedBlock{
				Validators: []validator.Set{
					setvalidator.SizeAtLeast(1),
				},
				NestedObject: schema.NestedBlockObject{
					Attributes: map[string]schema.Attribute{
						"role_arn": schema.StringAttribute{
							Optional: true,
							Computed: true,
						},
						"role_type": schema.StringAttribute{
							Optional: true,
							Computed: true,
						},
					},
				},
			},

Result on Apply:

╷
│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to aws_auditmanager_assessment.test, provider "provider[\"registry.terraform.io/hashicorp/aws\"]" produced an unexpected new value: .roles: actual set element
│ cty.ObjectVal(map[string]cty.Value{"role_arn":cty.StringVal("arn:aws:iam::012345678901:role/redacted-admin"), "role_type":cty.StringVal("PROCESS_OWNER")}) does not correlate with any element
│ in plan.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
╵
╷
│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to aws_auditmanager_assessment.test, provider "provider[\"registry.terraform.io/hashicorp/aws\"]" produced an unexpected new value: .roles: block set length changed from 1 to 2.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.

Set Attribute with Optional and Computed

Schema:

			"roles": schema.SetAttribute{
				Optional:    true,
				Computed:    true,
				ElementType: types.ObjectType{AttrTypes: assessmentRolesAttrTypes},
			},

Result on Apply:

│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to aws_auditmanager_assessment.test, provider "provider[\"registry.terraform.io/hashicorp/aws\"]" produced an unexpected new value: .roles: actual set element
│ cty.ObjectVal(map[string]cty.Value{"role_arn":cty.StringVal("arn:aws:iam::012345678901:role/redacted-admin"), "role_type":cty.StringVal("PROCESS_OWNER")}) does not correlate with any element
│ in plan.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
╵
╷
│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to aws_auditmanager_assessment.test, provider "provider[\"registry.terraform.io/hashicorp/aws\"]" produced an unexpected new value: .roles: length changed from 1 to 2.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.

Set Attribute with Optional and Computed, Plus Plan Modifiers

Schema:

			"roles": schema.SetAttribute{
				Optional:    true,
				Computed:    true,
				ElementType: types.ObjectType{AttrTypes: assessmentRolesAttrTypes},
				PlanModifiers: []planmodifier.Set{
					setplanmodifier.UseStateForUnknown(),
					newRolesPlanModifier(),
				},
			},

Plan Modifier:

type rolesPlanModifier struct{}

func (m rolesPlanModifier) Description(context.Context) string {
	return "Handles merging configured and computed roles set entries"
}

func (m rolesPlanModifier) MarkdownDescription(ctx context.Context) string {
	return m.Description(ctx)
}

func (m rolesPlanModifier) PlanModifySet(ctx context.Context, req planmodifier.SetRequest, resp *planmodifier.SetResponse) {
	// On create, set the plan to unknown as additional values may be returned alongside
	// those specified in the configuration
	if req.StateValue.IsNull() {
		resp.PlanValue = types.SetUnknown(types.ObjectType{AttrTypes: assessmentRolesAttrTypes})
		return
	}
	// On destroy, do nothing
	if req.PlanValue.IsNull() {
		return
	}

	// On update, merge the existing values in state with anything new from the configuration
	resp.PlanValue = mergeSets(req.StateValue, req.PlanValue)
}

func mergeSets(s1, s2 types.Set) types.Set {
	// TODO: implement
	return s1
}

func newRolesPlanModifier() planmodifier.Set {
	return rolesPlanModifier{}
}

Result on Apply:

│ Error: Provider produced invalid plan
│
│ Provider "registry.terraform.io/hashicorp/aws" planned an invalid value for aws_auditmanager_assessment.test.roles: planned value
│ cty.UnknownVal(cty.Set(cty.Object(map[string]cty.Type{"role_arn":cty.String, "role_type":cty.String}))) does not match config value
│ cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{"role_arn":cty.StringVal("arn:aws:iam::012345678901:role/jb-test2"), "role_type":cty.StringVal("PROCESS_OWNER")})}).
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.

Multiple SetNestedBlocks

With this approach, separate nested blocks were used to distinguish between user provided values (roles), and the complete list of roles with access to the assessment (roles_all).

Schema:

			"roles": schema.SetNestedBlock{
				Validators: []validator.Set{
					setvalidator.SizeAtLeast(1),
				},
				NestedObject: schema.NestedBlockObject{
					Attributes: map[string]schema.Attribute{
						"role_arn": schema.StringAttribute{
							Required: true,
						},
						"role_type": schema.StringAttribute{
							Required: true,
							Validators: []validator.String{
								enum.FrameworkValidate[awstypes.RoleType](),
							},
						},
					},
				},
			},
			"roles_all": schema.SetNestedBlock{
				PlanModifiers: []planmodifier.Set{
					setplanmodifier.UseStateForUnknown(),
				},
				NestedObject: schema.NestedBlockObject{
					Attributes: map[string]schema.Attribute{
						"role_arn": schema.StringAttribute{
							Computed: true,
						},
						"role_type": schema.StringAttribute{
							Computed: true,
						},
					},
				},
			},

Result on Apply:

│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to aws_auditmanager_assessment.test, provider "provider[\"registry.terraform.io/hashicorp/aws\"]" produced an unexpected new value: .roles_all: actual set element
│ cty.ObjectVal(map[string]cty.Value{"role_arn":cty.StringVal("arn:aws:iam::012345678901:role/jb-test2"), "role_type":cty.StringVal("PROCESS_OWNER")}) does not correlate with any element in plan.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
╵
╷
│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to aws_auditmanager_assessment.test, provider "provider[\"registry.terraform.io/hashicorp/aws\"]" produced an unexpected new value: .roles_all: actual set element
│ cty.ObjectVal(map[string]cty.Value{"role_arn":cty.StringVal("arn:aws:iam::012345678901:role/redacted-admin"), "role_type":cty.StringVal("PROCESS_OWNER")}) does not correlate with any element
│ in plan.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
╵
╷
│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to aws_auditmanager_assessment.test, provider "provider[\"registry.terraform.io/hashicorp/aws\"]" produced an unexpected new value: .roles_all: block set length changed from 0 to 2.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.

Multiple Set Attributes

With this approach, separate attributes were used to distinguish between user provided values (roles), and the complete list of roles with access to the assessment (roles_all).

Schema:

			"roles": schema.SetAttribute{
				Required:    true,
				ElementType: types.ObjectType{AttrTypes: assessmentRolesAttrTypes},
			},
			"roles_all": schema.SetAttribute{
				Computed:    true,
				ElementType: types.ObjectType{AttrTypes: assessmentRolesAttrTypes},
				PlanModifiers: []planmodifier.Set{
					setplanmodifier.UseStateForUnknown(),
				},
			},

Result on Apply: Success

Other Alternatives

Filter API Results on Create/Update

This would involve parsing the returned Roles object and only writing values that also exist in the configuration to state. This provides no drift detection for roles not tracked in the configuration. See this related issue comment.

Set Configuration only on Create/Update

This would involve ignoring the returned Roles object and writing the roles configuration directly to state. This provides no drift detection for roles not tracked in the configuration. See this related issue comment.

Reference

Related Issues

Tested Terraform Configuration

While testing a development configuration was used as the AuditManager assessment resource is not yet published.

main.tf:

terraform {
  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = "~> 4.0"
    }
  }
}

# Configure the AWS Provider
provider "aws" {}

data "aws_caller_identity" "current" {}

resource "aws_s3_bucket" "test" {
  bucket = "jb-test-auditmanager-reporting-destination"
}

resource "aws_s3_bucket_acl" "test" {
  bucket = aws_s3_bucket.test.id
  acl    = "private"
}

resource "aws_iam_role" "test" {
  name = "jb-test2"

  assume_role_policy = jsonencode({
    Version = "2012-10-17"
    Statement = [
      {
        Action = "sts:AssumeRole"
        Effect = "Allow"
        Sid    = ""
        Principal = {
          Service = "ec2.amazonaws.com"
        }
      },
    ]
  })
}

resource "aws_auditmanager_control" "test" {
  name = "jb-test"

  control_mapping_sources {
    source_name          = "jb-test"
    source_set_up_option = "Procedural_Controls_Mapping"
    source_type          = "MANUAL"
  }
}

resource "aws_auditmanager_framework" "test" {
  name = "jb-test"

  control_sets {
    name = "jb-test"
    controls {
      id = aws_auditmanager_control.test.id
    }
  }
}

resource "aws_auditmanager_assessment" "test" {
  depends_on = [aws_iam_role.test, aws_s3_bucket.test, aws_s3_bucket_acl.test]

  name = "jb-test"

  assessment_reports_destination {
    destination      = format("s3://%s", aws_s3_bucket.test.id)
    destination_type = "S3"
  }

  framework_id = aws_auditmanager_framework.test.id

  roles {
    role_arn  = aws_iam_role.test.arn
    role_type = "PROCESS_OWNER"
  }

  scope {
    aws_accounts {
      id = data.aws_caller_identity.current.account_id
    }
    aws_services {
      service_name = "S3"
    }
  }
}

dev.tfrc:

provider_installation {
  dev_overrides {
    "hashicorp/aws" = "/path/to/go/bin"
  }
  direct {}
}

Workflow to reproduce:

$ TF_CLI_CONFIG_FILE=dev.tfrc terraform apply
$ TF_CLI_CONFIG_FILE=dev.tfrc terraform plan
$ TF_CLI_CONFIG_FILE=dev.tfrc terraform destroy

Existing Resource Examples (SDKv2)

Kendra Index

"document_metadata_configuration_updates": {
Type: schema.TypeSet,
Computed: true,
Optional: true,
MinItems: 0,
MaxItems: 500,

References

Relates #17981
Relates #28356

Would you like to implement a fix?

None

@jar-b jar-b added the needs-triage Waiting for first response or review from a maintainer. label Jan 3, 2023
@github-actions
Copy link

github-actions bot commented Jan 3, 2023

Community Note

Voting for Prioritization

  • Please vote on this issue by adding a 👍 reaction to the original post to help the community and maintainers prioritize this request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

Volunteering to Work on This Issue

  • If you are interested in working on this issue, please leave a comment.
  • If this would be your first contribution, please review the contribution guide.

@github-actions github-actions bot added bug Addresses a defect in current functionality. service/auditmanager Issues and PRs that pertain to the auditmanager service. service/iam Issues and PRs that pertain to the iam service. service/s3 Issues and PRs that pertain to the s3 service. service/sts Issues and PRs that pertain to the sts service. and removed needs-triage Waiting for first response or review from a maintainer. labels Jan 3, 2023
@jar-b jar-b added service/auditmanager Issues and PRs that pertain to the auditmanager service. and removed bug Addresses a defect in current functionality. service/iam Issues and PRs that pertain to the iam service. service/s3 Issues and PRs that pertain to the s3 service. service/sts Issues and PRs that pertain to the sts service. service/auditmanager Issues and PRs that pertain to the auditmanager service. labels Jan 3, 2023
@github-actions github-actions bot added this to the v4.49.0 milestone Jan 4, 2023
@github-actions
Copy link

github-actions bot commented Jan 5, 2023

This functionality has been released in v4.49.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

github-actions bot commented Feb 5, 2023

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 Feb 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
service/auditmanager Issues and PRs that pertain to the auditmanager service.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant