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

[Bug]: #26878

Open
n3mawashi opened this issue Sep 20, 2022 · 7 comments
Open

[Bug]: #26878

n3mawashi opened this issue Sep 20, 2022 · 7 comments
Labels
service/backup Issues and PRs that pertain to the backup service. stale Old or inactive issues managed by automation, if no further action taken these will get closed.

Comments

@n3mawashi
Copy link

Terraform Core Version

0.1.12

AWS Provider Version

4.20.1

Affected Resource(s)

Hi - I ran into a minor bug where aws_backup_framework always expects the input parameter value to be a string. This problem is that this control requires a list or ARNs as per AWS documentation of https://docs.aws.amazon.com/config/latest/developerguide/backup-recovery-point-manual-deletion-disabled.html

To reproduce the bug

resource "aws_backup_framework" "example_framework" {
  control {
    name = "BACKUP_RECOVERY_POINT_MANUAL_DELETION_DISABLED"
      input_parameter {
        name = "principalArnList" 
        value = [
          "arn:aws:iam::*:role/SysAdminRole",
         "arn:aws:iam::*:role/OtherRole"
      ]
   }
}

This produces an error

Error: Incorrect attribute value type
│
│   on ../modules/backup/backup-audit.tf line 6, in resource "aws_backup_framework" "example":
│   6:       value = ["arn:aws:iam::*:role/SysAdminRole", "arn:aws:iam::*:role/OtherRole"]
│
│ Inappropriate value for attribute "value": string required.

If I try to craft a string with escaped square bracket and commas I get the following message from the AWS API

Error: error updating Backup Framework (example): InvalidParameterValueException: Invalid value for the parameter principalArnList in control BACKUP_RECOVERY_POINT_MANUAL_DELETION_DISABLED. Expected comma-separated list of valid IAM principal ARNs.
│ {
│   RespMetadata: {
│     StatusCode: 400,
│     RequestID: "c1ed3eb5-f13f-4801-97fe-b900ae609f03"
│   },
│   Message_: "Invalid value for the parameter principalArnList in control BACKUP_RECOVERY_POINT_MANUAL_DELETION_DISABLED. Expected comma-separated list of valid IAM principal ARNs."
│ }

I think the issue stems from the following:
https://github.com/hashicorp/terraform-provider-aws/blob/main/internal/service/backup/framework.go#L56

My go experience is nonexistent, but I'll try to come up with a solution.

Expected Behavior

For the ARN list to be applied as input a parameter

Actual Behavior

Errors

Error: Incorrect attribute value type
│
│   on ../modules/backup/backup-audit.tf line 6, in resource "aws_backup_framework" "example":
│   6:       value = ["arn:aws:iam::*:role/SysAdminRole", "arn:aws:iam::*:role/OtherRole"]
│
│ Inappropriate value for attribute "value": string required.

Relevant Error/Panic Output Snippet

No response

Terraform Configuration Files

resource "aws_backup_framework" "example_framework" {
  control {
    name = "BACKUP_RECOVERY_POINT_MANUAL_DELETION_DISABLED"
      input_parameter {
        name = "principalArnList" 
        value = [
          "arn:aws:iam::*:role/SysAdminRole",
         "arn:aws:iam::*:role/OtherRole"
      ]
   }
}

Steps to Reproduce

  1. Add the above code to main.tf with provider code
  2. assume a role.
  3. run tf plan

Debug Output

No response

Panic Output

No response

Important Factoids

No response

References

No response

Would you like to implement a fix?

No

@n3mawashi n3mawashi added bug Addresses a defect in current functionality. needs-triage Waiting for first response or review from a maintainer. labels Sep 20, 2022
@github-actions github-actions bot added service/backup Issues and PRs that pertain to the backup service. and removed bug Addresses a defect in current functionality. labels Sep 20, 2022
@github-actions
Copy link

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.

@justinretzolk
Copy link
Member

Hey @n3mawashi 👋 Thank you for taking the time to raise this! I believe the issue you're seeing here is that you're trying to pass multiple values to a single input_parameter. If you instead break them up into multiple input_parameter blocks, they should merge with the result that you're expecting. This would look something like the following (full disclosure: I'm not able to easily test this to verify, and have based this off of double checking the documentation and schema):

resource "aws_backup_framework" "example_framework" {
  name = "example-framework"
  
  control {
    name = "BACKUP_RECOVERY_POINT_MANUAL_DELETION_DISABLED"
      
    input_parameter {
      name = "principalArnList" 
      value = "arn:aws:iam::*:role/SysAdminRole"
    }

    input_parameter {
      name = "principalArnList" 
      value = "arn:aws:iam::*:role/OtherRole"
    }
  }
}

@justinretzolk justinretzolk added waiting-response Maintainers are waiting on response from community or contributor. and removed needs-triage Waiting for first response or review from a maintainer. labels Sep 20, 2022
@n3mawashi
Copy link
Author

Thanks, @justinretzolk for the quick reply. Unfortunately, that hasn't worked. still getting the same error of

 Error: error updating Backup Framework (icare_uat_compliance_framework): InvalidParameterValueException: Invalid value for the parameter principalArnList in control BACKUP_RECOVERY_POINT_MANUAL_DELETION_DISABLED. Expected comma-separated list of valid IAM principal ARNs.
│ {
│   RespMetadata: {
│     StatusCode: 400,
│     RequestID: "4b6d4489-67e4-4648-9e00-dc22847cbf15"
│   },
│   Message_: "Invalid value for the parameter principalArnList in control BACKUP_RECOVERY_POINT_MANUAL_DELETION_DISABLED. Expected comma-separated list of valid IAM principal ARNs."
│ }
│ 
│   with module.backup.aws_backup_framework.compliance_framework[0],
│   on ../modules/backup/backup-audit.tf line 1, in resource "aws_backup_framework" "compliance_framework":
│    1: resource "aws_backup_framework" "compliance_framework" {
│ 
╵

@github-actions github-actions bot removed the waiting-response Maintainers are waiting on response from community or contributor. label Sep 21, 2022
@n3mawashi
Copy link
Author

here is the modify control code that I used

  control {
    name = "BACKUP_RECOVERY_POINT_MANUAL_DELETION_DISABLED"
    input_parameter {
      name = "principalArnList" 
      value = "arn:aws:iam::*:role/SysAdminRole"
    }
    input_parameter {
      name = "principalArnList" 
      value = "arn:aws:iam::*:role/OtherAdmin"  
    }

    scope {
      tags = {
        name = var.backup_tag_selection
        value = "true"
      }
    }
  }
}

@justinretzolk
Copy link
Member

Hey @n3mawashi 👋 Thank you for the followup, and again, sorry that I'm not able to easily test this to have caught that error! I did a bit more digging and found that I was mistaken about the multiple input_parameter blocks merging since they had the same name value -- that does not appear to happen.

I did a bit of additional testing, ran the following through terraform validate, and it passed, so I suspect it may work. The key thing here was that the input_parameter.value is a string, so will pass Terraform validation. The underlying Go Type in the SDK expects as string as well, so my assumption here is that the API receives this as a string and then converts it. I'm unsure of whether the API expects the ARNs in the list to be wrapped in (escaped) quotes or not, so I provided examples of each way below.

Without wrapping the ARNs in quotes:

    input_parameter {
      name = "principalArnList" 
      value = "[arn:aws:iam::*:role/SysAdminRole, arn:aws:iam::*:role/OtherRole]"
    }

Wrapping the ARNs in quotes:

    input_parameter {
      name = "principalArnList" 
      value = "[\"arn:aws:iam::*:role/SysAdminRole\", \"arn:aws:iam::*:role/OtherRole\"]"
    }

@justinretzolk justinretzolk added the waiting-response Maintainers are waiting on response from community or contributor. label Sep 21, 2022
@mattburgess
Copy link
Collaborator

Just to interject here, but I think the docs the OP linked to actually state what it's after, which is just a CSV string, so I reckon this should work:

    input_parameter {
      name = "principalArnList" 
      value = "arn:aws:iam::*:role/SysAdminRole,arn:aws:iam::*:role/OtherRole"
    }

@github-actions github-actions bot removed the waiting-response Maintainers are waiting on response from community or contributor. label Oct 6, 2022
Copy link

Marking this issue as stale due to inactivity. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 30 days it will automatically be closed. Maintainers can also remove the stale label.

If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thank you!

@github-actions github-actions bot added the stale Old or inactive issues managed by automation, if no further action taken these will get closed. label Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service/backup Issues and PRs that pertain to the backup service. stale Old or inactive issues managed by automation, if no further action taken these will get closed.
Projects
None yet
Development

No branches or pull requests

3 participants