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 aws Shield feature request 1769 (Part I) #7591

Merged
merged 2 commits into from
Feb 19, 2019

Conversation

parabolic
Copy link
Contributor

@parabolic parabolic commented Feb 18, 2019

This is a separate PR that adds the package from the aws-sdk-go as it is written in the documentation https://github.com/terraform-providers/terraform-provider-aws

If you need to add a new package in the vendor directory under github.com/aws/aws-sdk-go, create a separate PR handling only the update of the vendor for your new requirement. Make sure to pin your dependency to a specific version, and that all versions of github.com/aws/aws-sdk-go/* are pinned to the same version.

The following commands where used:

go mod tidy
go mod vendor

Fixes #1769

Changes proposed in this pull request:

The shield service is present in the aws-sdk-go since release Release v1.16.28 (2019-02-05) but judging from the PR that @bflad committed the shield service was not actually pushed.
In order to have this resource in terraform we would need to have the aws-sdk-go package first.
ffd5a63

This PR supersedes #1899

the documentation from https://github.com/terraform-providers/terraform-provider-aws
the following commands where used:
go mod tidy
go mod vendor
@ghost ghost added size/XXL Managed by automation to categorize the size of a PR. dependencies Used to indicate dependency changes. labels Feb 18, 2019
@bflad bflad added the service/shield Issues and PRs that pertain to the shield service. label Feb 19, 2019
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Hi @parabolic 👋 Thanks for submitting this. Since we are now working with Go Modules, getting new dependencies to "stick" now actually requires some implementation of the new dependency.

By example here, if I pull down this pull request locally with Go 1.11.5 outside the GOPATH, Go will automatically remove this dependency as its unused:

$ git checkout -b parabolic-1769_feature_request_add_shield master
$ git pull https://github.com/parabolic/terraform-provider-aws.git 1769_feature_request_add_shield
$ go mod tidy
$ go mod vendor
$ git status
On branch parabolic-1769_feature_request_add_shield
Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	deleted:    vendor/github.com/aws/aws-sdk-go/service/shield/api.go
	deleted:    vendor/github.com/aws/aws-sdk-go/service/shield/doc.go
	deleted:    vendor/github.com/aws/aws-sdk-go/service/shield/errors.go
	deleted:    vendor/github.com/aws/aws-sdk-go/service/shield/service.go
	modified:   vendor/modules.txt

The simplest solution here is to instantiate the AWS Go SDK service client in aws/config.go, similar to #7433:

// in imports
"github.com/aws/aws-sdk-go/service/shield"
// in AWSClient struct
shieldconn              *shield.Shield
// with other client creations
client.shieldconn = shield.New(sess)

Cheers 👍

@bflad bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Feb 19, 2019
@ghost ghost added the provider Pertains to the provider itself, rather than any interaction with AWS. label Feb 19, 2019
@parabolic
Copy link
Contributor Author

Hi @bflad thanks for the explanation!

I've instantiated the service as advised.

Cheers!

@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Feb 19, 2019
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @parabolic! 🚀

@bflad bflad merged commit 5cbb88f into hashicorp:master Feb 19, 2019
@bflad bflad added this to the v1.60.0 milestone Feb 19, 2019
@ghost
Copy link

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

@ghost ghost locked and limited conversation to collaborators Mar 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dependencies Used to indicate dependency changes. provider Pertains to the provider itself, rather than any interaction with AWS. service/shield Issues and PRs that pertain to the shield service. size/XXL Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants