New Resource: PublicDnsNamespace #2569

atsushi-ishibashi · 2017-12-06T15:29:21Z

No description provided.

atsushi-ishibashi · 2017-12-07T15:53:09Z

TF_ACC=1 go test ./aws -v -run=TestAccAwsServiceDiscoveryPublicDnsNamespace_basic -timeout 120m
=== RUN   TestAccAwsServiceDiscoveryPublicDnsNamespace_basic
--- PASS: TestAccAwsServiceDiscoveryPublicDnsNamespace_basic (112.03s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	112.077s

atsushi-ishibashi · 2017-12-07T15:56:17Z

Ready to review👍

radeksimko

Hey @atsushi-ishibashi
thanks for the PR.

I left you a few comments there, nothing major though.

radeksimko · 2017-12-11T13:23:51Z

aws/resource_aws_service_discovery_public_dns_namespace_test.go

+			{
+				Config: testAccServiceDiscoveryPublicDnsNamespaceConfig(acctest.RandString(5)),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAwsServiceDiscoveryPublicDnsNamespaceExists("aws_service_discovery_public_dns_namespace.test"),


Can you also add 2 more checks here for the computed fields, please? i.e.

resource.TestCheckResourceAttrSet("aws_service_discovery_public_dns_namespace.test", "arn"), resource.TestCheckResourceAttrSet("aws_service_discovery_public_dns_namespace.test", "hosted_zone"),

radeksimko · 2017-12-11T13:30:20Z

aws/resource_aws_service_discovery_public_dns_namespace.go

+		input.Description = aws.String(v.(string))
+	}
+
+	resp, err := conn.CreatePublicDnsNamespace(input)


Shouldn't we also use CreatorRequestId to make it safe to create multiple resources in parallel?

We usually use randomly generated string, like here:
https://github.com/terraform-providers/terraform-provider-aws/blob/0c0887cff37a40c1d35ee2bbe1788831d1f871e2/aws/resource_aws_mq_broker.go#L184

radeksimko · 2017-12-11T13:38:11Z

aws/resource_aws_service_discovery_public_dns_namespace.go

+		Refresh:    servicediscoveryOperationRefreshStatusFunc(conn, *resp.OperationId),
+		Timeout:    5 * time.Minute,
+		Delay:      10 * time.Second,
+		MinTimeout: 3 * time.Second,


Is there any particular reason for delaying the refresh here? i.e. do the two fields Delay & MinTimeout have any reason? Based on my brief testing the deletion can finish in 2 seconds, so we're potentially letting the user wait for 8 more seconds unnecessarily here.

radeksimko · 2017-12-11T13:38:55Z

aws/resource_aws_service_discovery_public_dns_namespace.go

+		Refresh:    servicediscoveryOperationRefreshStatusFunc(conn, *resp.OperationId),
+		Timeout:    5 * time.Minute,
+		Delay:      10 * time.Second,
+		MinTimeout: 3 * time.Second,


Is there any particular reason for delaying the refresh here? i.e. do the two fields Delay & MinTimeout have any reason? Based on my brief testing the creation can finish in 2 seconds, so we're potentially letting the user wait for 8 more seconds unnecessarily here.

radeksimko · 2017-12-11T13:46:01Z

aws/resource_aws_service_discovery_public_dns_namespace.go

+	d.Set("description", resp.Namespace.Description)
+	d.Set("arn", resp.Namespace.Arn)
+	if resp.Namespace.Properties != nil {
+		d.Set("hosted_zone", resp.Namespace.Properties.DnsProperties.HostedZoneId)


I wonder if Amazon has plans for expanding Namespace.Properties or Namespace.Properties.DnsProperties.

It's no big deal though as we can always deprecate this 1st-level field in favour of its nested version if it becomes necessary.

atsushi-ishibashi · 2017-12-11T14:57:34Z

@radeksimko Ok👍

TF_ACC=1 go test ./aws -v -run=TestAccAwsServiceDiscoveryPublicDnsNamespace_basic -timeout 120m
=== RUN   TestAccAwsServiceDiscoveryPublicDnsNamespace_basic
--- PASS: TestAccAwsServiceDiscoveryPublicDnsNamespace_basic (65.66s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	65.742s

* 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

* Makefile: Add sweep target * Add more example and missing field * 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 * hashicorp#2217: re-enable default encryption after disabling in via UI * hashicorp#2217: documentation

ghost · 2020-04-10T17:18:16Z

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!

atsushi-ishibashi changed the title ~~[WIP]New Resource: PublicDnsNamespace~~ New Resource: PublicDnsNamespace Dec 7, 2017

atsushi-ishibashi added 2 commits December 8, 2017 00:55

WIP

61e464f

Add test, docs

5089f69

atsushi-ishibashi force-pushed the public_dns_namespace branch from 8231a68 to 5089f69 Compare December 7, 2017 15:55

This was referenced Dec 8, 2017

New Resource: ServiceDiscovery PrivateDNS Namespace #2589

Merged

New Resource: Service Discovery Service #2613

Merged

radeksimko reviewed Dec 11, 2017

View reviewed changes

radeksimko added waiting-response Maintainers are waiting on response from community or contributor. new-resource Introduces a new resource. labels Dec 11, 2017

Reflect reviews

8fb54ba

Modify error handling

16ffc02

radeksimko removed the waiting-response Maintainers are waiting on response from community or contributor. label Dec 12, 2017

radeksimko merged commit 3fc4ab2 into hashicorp:master Dec 12, 2017

atsushi-ishibashi deleted the public_dns_namespace branch December 13, 2017 15:16

atsushi-ishibashi mentioned this pull request Dec 19, 2017

Support Route53 Auto Naming #2561

Closed

ghost locked and limited conversation to collaborators Apr 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: PublicDnsNamespace #2569

New Resource: PublicDnsNamespace #2569

atsushi-ishibashi commented Dec 6, 2017

atsushi-ishibashi commented Dec 7, 2017

atsushi-ishibashi commented Dec 7, 2017

radeksimko left a comment

radeksimko Dec 11, 2017

radeksimko Dec 11, 2017

radeksimko Dec 11, 2017

radeksimko Dec 11, 2017

radeksimko Dec 11, 2017

atsushi-ishibashi commented Dec 11, 2017

ghost commented Apr 10, 2020

New Resource: PublicDnsNamespace #2569

New Resource: PublicDnsNamespace #2569

Conversation

atsushi-ishibashi commented Dec 6, 2017

atsushi-ishibashi commented Dec 7, 2017

atsushi-ishibashi commented Dec 7, 2017

radeksimko left a comment

Choose a reason for hiding this comment

radeksimko Dec 11, 2017

Choose a reason for hiding this comment

radeksimko Dec 11, 2017

Choose a reason for hiding this comment

radeksimko Dec 11, 2017

Choose a reason for hiding this comment

radeksimko Dec 11, 2017

Choose a reason for hiding this comment

radeksimko Dec 11, 2017

Choose a reason for hiding this comment

atsushi-ishibashi commented Dec 11, 2017

ghost commented Apr 10, 2020