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

Add name_prefix to aws_iam_instance_profile and aws_iam_role #6939

Merged
merged 1 commit into from
Jun 4, 2016
Merged

Add name_prefix to aws_iam_instance_profile and aws_iam_role #6939

merged 1 commit into from
Jun 4, 2016

Conversation

joshuaspence
Copy link
Contributor

@joshuaspence joshuaspence commented May 30, 2016

Ref #6099. Add a name_prefix attribute to the aws_iam_instance_profile and aws_iam_role resources.

@stack72
Copy link
Contributor

stack72 commented Jun 2, 2016

Hi @joshuaspence

when you said that it doesn't quite work yet, what do you mean? :)

P;

@joshuaspence
Copy link
Contributor Author

Oh, the acceptance tests are failing and I don't understand why.

ForceNew: true,
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

@joshuaspence as you do not specify a name but do specify a name_prefix, you need to make name Computed. This means that when we set it in the Read, we will not get any alternative plan

Doing this locally gave me this:

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSIAMInstanceProfile'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSIAMInstanceProfile -timeout 120m
=== RUN   TestAccAWSIAMInstanceProfile_basic
--- PASS: TestAccAWSIAMInstanceProfile_basic (16.34s)
=== RUN   TestAccAWSIAMInstanceProfile_namePrefix
--- PASS: TestAccAWSIAMInstanceProfile_namePrefix (17.01s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    33.367s

@stack72
Copy link
Contributor

stack72 commented Jun 2, 2016

@joshuaspence ok, 2 small changes needed here and we are in a working situation :)

P.

@stack72 stack72 changed the title Add name_prefix to aws_iam_instance_profile and aws_iam_role [WIP] Add name_prefix to aws_iam_instance_profile and aws_iam_role Jun 2, 2016
@stack72 stack72 self-assigned this Jun 2, 2016
@stack72 stack72 added the waiting-response An issue/pull request is waiting for a response from the community label Jun 3, 2016
@joshuaspence
Copy link
Contributor Author

Thanks, this should be ready to go now. I didn't quite understand why the name field needs to be marked as computed because it may not necessarily be so (for example, if the name attribute of the prefix is specified rather than name_prefix).

@joshuaspence joshuaspence changed the title [WIP] Add name_prefix to aws_iam_instance_profile and aws_iam_role Add name_prefix to aws_iam_instance_profile and aws_iam_role Jun 4, 2016
ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
// https://github.com/boto/botocore/blob/2485f5c/botocore/data/iam/2010-05-08/service-2.json#L8196-L8201
value := v.(string)
if len(value) > 64 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This value probably isn't correct... are there any guarantees made about the length of the string returned by the uuidV4 function?

@stack72
Copy link
Contributor

stack72 commented Jun 4, 2016

Hi @joshuaspence

I'll give this a final set of tests when I am back online. Just a note on Computed - this is used when there is a chance that the value get's set on the Read but not in the initial config. The Computed means that Terraform will understand why there would be changes in that case

P.

@stack72
Copy link
Contributor

stack72 commented Jun 4, 2016

@joshuaspence these looks great now :)

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSRole'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSRole -timeout 120m
=== RUN   TestAccAWSRolePolicyAttachment_basic
--- PASS: TestAccAWSRolePolicyAttachment_basic (33.34s)
=== RUN   TestAccAWSRole_basic
--- PASS: TestAccAWSRole_basic (16.32s)
=== RUN   TestAccAWSRole_namePrefix
--- PASS: TestAccAWSRole_namePrefix (16.08s)
=== RUN   TestAccAWSRole_testNameChange
--- PASS: TestAccAWSRole_testNameChange (39.93s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    105.703s
make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSIAMInstanceProfile'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSIAMInstanceProfile -timeout 120m
=== RUN   TestAccAWSIAMInstanceProfile_basic
--- PASS: TestAccAWSIAMInstanceProfile_basic (18.32s)
=== RUN   TestAccAWSIAMInstanceProfile_namePrefix
--- PASS: TestAccAWSIAMInstanceProfile_namePrefix (19.99s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    38.329s

Thanks for the hard work!

@stack72 stack72 merged commit d3030e1 into hashicorp:master Jun 4, 2016
@joshuaspence joshuaspence deleted the name_prefix-iam branch April 2, 2017 03:14
@ghost
Copy link

ghost commented Apr 14, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement provider/aws waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants