r/aws_elasticache_security_group: add import support #2277

bflad · 2017-11-14T08:53:47Z

Sorry I cannot run TestAccAWSElasticacheSecurityGroup_Import as my AWS account has its default VPCs removed.

radeksimko · 2017-11-15T11:51:50Z

aws/resource_aws_elasticache_security_group.go

@@ -17,6 +17,12 @@ func resourceAwsElasticacheSecurityGroup() *schema.Resource {
 		Create: resourceAwsElasticacheSecurityGroupCreate,
 		Read:   resourceAwsElasticacheSecurityGroupRead,
 		Delete: resourceAwsElasticacheSecurityGroupDelete,
+		Importer: &schema.ResourceImporter{
+			State: func(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
+				d.Set("name", d.Id())


Instead of adding a custom importer, how do you feel about replacing d.Get("name") with d.Id()? It seems a bit cleaner as we may then use the simple passthrough importer here.

bflad · 2017-11-15T12:24:20Z

Personally, I'd prefer d.Id() and the pass through importer to match most of the other resources. I was just submitting the smallest change for the functionality. 😄

Can you verify the acceptance testing passes in your account as-is first?

There are a number of resources that have a similar issue in their read function. Maybe it'd be worth doing a pass through them to convert, which I don't mind doing.

Would you prefer bundling the refactoring of this one in this PR now or them all together? I can create a github issue either which way. Thanks!

radeksimko · 2017-11-15T13:01:48Z

Can you verify the acceptance testing passes in your account as-is first?

It does not and I believe it's because during import Terraform won't look at the static provider block (which rightly contains us-east-1 where we have EC2Classic enabled), it expects it to be configured from outside. Here's an example of how to do that from within the test:

	oldRegion := os.Getenv("AWS_DEFAULT_REGION")
	os.Setenv("AWS_DEFAULT_REGION", "us-east-1")
	defer os.Setenv("AWS_DEFAULT_REGION", oldRegion)

you can find more examples in other tests if you search for AWS_DEFAULT_REGION.

Would you prefer bundling the refactoring of this one in this PR now or them all together?

I'd prefer 1 PR per resource, if possible. Smaller PRs are much easier to review.

Thanks.

radeksimko · 2017-11-15T13:02:55Z

btw. when I add the region workaround I'm still getting failure:

=== RUN   TestAccAWSElasticacheSecurityGroup_Import
--- FAIL: TestAccAWSElasticacheSecurityGroup_Import (40.75s)
	testing.go:503: Step 1 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.

		(map[string]string) {
		}


		(map[string]string) (len=2) {
		 (string) (len=22) "security_group_names.#": (string) (len=1) "1",
		 (string) (len=31) "security_group_names.4153974498": (string) (len=41) "tf-test-security-group-653022196438882106"
		}

bflad · 2017-11-28T02:03:54Z

Sorry for my slow response here. 😦 I'll convert this over to d.Id() and fix up the test right now.

* Use d.Id() instead of d.Get("name") on read, which allows using schema.ImportStatePassthrough * d.Set("security_group_names") on read * Set AWS_DEFAULT_REGION to us-east-1 on import testing

bflad · 2017-11-28T02:51:25Z

Ah, the test failure previously was due to a lack of setting the security group names in the state during read, fixed! Now we're also using d.Id() on read with ImportStatePassthrough for import. With the addition of the import test EC2-Classic workaround, this verified locally for me (just so happens I have an EC2-Classic enabled us-east-1 account I can use). 👍

make testacc TEST=./aws TESTARGS='-run=TestAccAWSElasticacheSecurityGroup'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSElasticacheSecurityGroup -timeout 120m
=== RUN   TestAccAWSElasticacheSecurityGroup_basic
--- PASS: TestAccAWSElasticacheSecurityGroup_basic (9.18s)
=== RUN   TestAccAWSElasticacheSecurityGroup_Import
--- PASS: TestAccAWSElasticacheSecurityGroup_Import (7.06s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	16.281s

radeksimko

👍

* r/aws_elasticache_security_group: add import support * r/aws_elasticache_security_group: hashicorp#2277 review updates * Use d.Id() instead of d.Get("name") on read, which allows using schema.ImportStatePassthrough * d.Set("security_group_names") on read * Set AWS_DEFAULT_REGION to us-east-1 on import testing

* Add Data Source: aws_elb * Fix dataSourceAwsElb typo * Fix dataSourceAwsElb Schema name field to not include Computed * Remove dataSourceAwsElb schema defaults for computed fields * Remove dataSourceAwsElb schema defaults for nested computed fields too * Corrected depends_on entry for EIP depends_on = ["aws_internet_gateway.gw"] is the correct syntax * Add sweeper for IAM Server Certificates * test/aws_config_delivery_channel: Add missing dependencies * d/aws_elb r/aws_elb: hashicorp#2004 review comments * Remove enable_deletion_protection from testAccDataSourceAWSELBConfigBasic * Replace unnecessary errwrap.Wrapf with fmt.Errorf * Reduce flattenAwsELbResource to ec2conn and elbconn instead of meta * Properly name TestAccDataSourceAWSELB_basic resources * Use t.Name() for description and TestName tags * d/aws_elb: Fix documentation sidebar ordering after merging master with new d/aws_elasticache_replication_group * Makefile: Add sweep target * Update cognito_user_pool.markdown * Update CHANGELOG.md * r/aws_elasticache_security_group: add import support (hashicorp#2277) * r/aws_elasticache_security_group: add import support * r/aws_elasticache_security_group: hashicorp#2277 review updates * Use d.Id() instead of d.Get("name") on read, which allows using schema.ImportStatePassthrough * d.Set("security_group_names") on read * Set AWS_DEFAULT_REGION to us-east-1 on import testing * Update CHANGELOG.md * documentation: remove antislashes in page titles * Added missing WARN debug lines when reading a non-existing resource * Removed <wbr> from documentation titles * vendor: Bump aws-sdk-go to v.1.12.44 * Add logs for iam server certificate delete conflict (hashicorp#2533) * Query elb API for load balancer arn causing delete conflict - For IAM server certificate. * Use regex for lb name. * Edits for hashicorp#2533 * r/aws_sqs_queue_policy: Support import by queue URL (hashicorp#2544) * Update CHANGELOG.md * r/aws_elasticsearch_domain: Add LogPublishingOption (hashicorp#2285) * WIP * Add enabled * Use cwl policy * Reflect reviews * Update CHANGELOG.md * Add force_destroy field to aws_athena_database (hashicorp#2363) * Add force_destroy field to aws_athena_database. Fixes hashicorp#2362. * Remove unnecessary import. * Code review feedback * Update CHANGELOG.md * Add more example and missing field * New Resource: aws_media_store_container (hashicorp#2448) * New Resource: aws_media_store_container * Reflect reviews * remove policy * Update CHANGELOG.md * Add Redis AUTH, in-transit and at-rest encryption (hashicorp#2090) * add AUTH, at-rest and in-transit encryption to Elasticache replication groups * add _enabled to transit/at_rest encyrption parameters * added one more _enabled * move validateAwsElastiCacheReplicationGroupAuthToken to aws/validators.go, as well as tests * set auth_token to nil during Reads * update Replication Group encryption acceptance tests to use config functions instead of vars * Fix whitespacing (tabs -> spaces) * docs/elasticache_replication_group: Add missing fields * Update CHANGELOG.md * r/aws_dynamodb_table: Ensure ttl is properly read (hashicorp#2452) * r/aws_dynamodb_table: Ensure ttl is properly read * r/aws_dynamodb_table: hashicorp#2452 review updates * Add timeToLiveOutput.TimeToLiveDescription nil check * Simplify logic to d.Set ttl * Update CHANGELOG.md * Bump aws-sdk-go to v.1.12.45 * New Resource: PublicDnsNamespace (hashicorp#2569) * WIP * Add test, docs * Reflect reviews * Modify error handling * Update CHANGELOG.md * New Resource: ServiceDiscovery PrivateDNS Namespace (hashicorp#2589) * New Resource: service_discovery_private_dns_namespace * Reflect reviews * Update CHANGELOG.md

ghost · 2020-04-10T17:22:59Z

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

r/aws_elasticache_security_group: add import support

787064d

bflad mentioned this pull request Nov 14, 2017

Unable to import aws_elasticache_security_group #2260

Closed

radeksimko reviewed Nov 15, 2017

View reviewed changes

radeksimko added enhancement Requests to existing resources that expand the functionality or scope. waiting-response Maintainers are waiting on response from community or contributor. size/S Managed by automation to categorize the size of a PR. labels Nov 15, 2017

r/aws_elasticache_security_group: hashicorp#2277 review updates

6d7715a

* Use d.Id() instead of d.Get("name") on read, which allows using schema.ImportStatePassthrough * d.Set("security_group_names") on read * Set AWS_DEFAULT_REGION to us-east-1 on import testing

radeksimko approved these changes Dec 8, 2017

View reviewed changes

radeksimko merged commit 851b0e0 into hashicorp:master Dec 8, 2017

ghost locked and limited conversation to collaborators Apr 10, 2020

breathingdust removed the waiting-response Maintainers are waiting on response from community or contributor. label Sep 17, 2021

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

r/aws_elasticache_security_group: add import support #2277

r/aws_elasticache_security_group: add import support #2277

bflad commented Nov 14, 2017

radeksimko Nov 15, 2017

bflad commented Nov 15, 2017

radeksimko commented Nov 15, 2017 •

edited

Loading

radeksimko commented Nov 15, 2017

bflad commented Nov 28, 2017

bflad commented Nov 28, 2017

radeksimko left a comment

ghost commented Apr 10, 2020

r/aws_elasticache_security_group: add import support #2277

r/aws_elasticache_security_group: add import support #2277

Conversation

bflad commented Nov 14, 2017

radeksimko Nov 15, 2017

Choose a reason for hiding this comment

bflad commented Nov 15, 2017

radeksimko commented Nov 15, 2017 • edited Loading

radeksimko commented Nov 15, 2017

bflad commented Nov 28, 2017

bflad commented Nov 28, 2017

radeksimko left a comment

Choose a reason for hiding this comment

ghost commented Apr 10, 2020

radeksimko commented Nov 15, 2017 •

edited

Loading