-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
provider/aws: Add new resource - aws_vpc_endpoint #2695
provider/aws: Add new resource - aws_vpc_endpoint #2695
Conversation
return fmt.Errorf("VPC Endpoint not found") | ||
} | ||
|
||
*endpoint = *resp.VPCEndpoints[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I never understood what's the point of this construct which appears in most test*Exists
helpers, it doesn't seem to do anything...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@radeksimko it's an "out parameter" - we store the endpoint in the provided pointer to allow subsequent checks to inspect the object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, but we don't do any subsequent checks in some places where this is used (including here), so is it just "code for the future" ™️ that we want to keep in place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Answering to myself - I guess we should be checking attributes most of the time.
two tiny tiny nits. looking great! merge at will. |
22b3551
to
9114072
Compare
9114072
to
3c6fb8d
Compare
3c6fb8d
to
4d1d4ea
Compare
Fixed both and force-pushed, here's the diff: diff --git a/builtin/providers/aws/resource_aws_route_table.go b/builtin/providers/aws/resource_aws_route_table.go
index a8a44e0..f6d6313 100644
--- a/builtin/providers/aws/resource_aws_route_table.go
+++ b/builtin/providers/aws/resource_aws_route_table.go
@@ -149,6 +149,8 @@ func resourceAwsRouteTableRead(d *schema.ResourceData, meta interface{}) error {
}
if r.DestinationPrefixListID != nil {
+ // Skipping because VPC endpoint routes are handled separately
+ // See aws_vpc_endpoint
continue
}
diff --git a/builtin/providers/aws/resource_aws_vpc_endpoint.go b/builtin/providers/aws/resource_aws_vpc_endpoint.go
index 11c58ac..5617faa 100644
--- a/builtin/providers/aws/resource_aws_vpc_endpoint.go
+++ b/builtin/providers/aws/resource_aws_vpc_endpoint.go
@@ -133,8 +133,6 @@ func resourceAwsVPCEndpointUpdate(d *schema.ResourceData, meta interface{}) erro
if d.HasChange("policy") {
policy := normalizeJson(d.Get("policy"))
input.PolicyDocument = aws.String(policy)
- } else {
- input.ResetPolicy = aws.Boolean(false)
}
log.Printf("[DEBUG] Updating VPC Endpoint: %#v", input) |
provider/aws: Add new resource - aws_vpc_endpoint
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. |
This is mostly @gjohnson 's work which I took and polished & added tests + docs.
The authorship the actual resource is still credited to you, @gjohnson 😉
This should help us to close the following:
Test plan
I used
normalizeJson
from s3_bucket resource, do you want me to move it to a special file, so it's more obvious that it's not just used in s3_bucket? It feels a bit cleaner to do it, but it's still only a single function with a few lines.