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 Authorization Rules for Client VPN #13950

Merged
merged 50 commits into from
Jul 8, 2020
Merged
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
d8912bc
initial commit
slapula Feb 12, 2019
bbd7315
Merge branch 'master' into resource-aws-ec2-client-vpn-authorization-…
slapula Feb 12, 2019
a97816b
another progess commit
slapula Feb 12, 2019
ef5cba6
r/aws_ec2_client_vpn_endpoint: Reworked endpoint resource to include …
slapula Feb 14, 2019
598d1cb
updating doc with network_association
slapula Feb 14, 2019
6638498
more acc test tweaks
slapula Feb 15, 2019
7995ce7
more acctest changes
slapula Feb 16, 2019
91e12b9
Merge branch 'master' into resource-aws-ec2-client-vpn-consolidation
slapula Mar 14, 2019
d0c9bbf
adding route functionality
slapula Mar 15, 2019
20f36e2
Merge branch 'master' into resource-aws-ec2-client-vpn-consolidation
slapula Mar 16, 2019
f7fa8f4
readding network assoc resource w/ deprecation message
slapula Mar 17, 2019
6cc4c31
re-adding network association docs too
slapula Mar 17, 2019
5132423
renaming net assoc refresh func
slapula Mar 17, 2019
88362fb
Merge branch 'master' into resource-aws-ec2-client-vpn-consolidation
slapula Aug 13, 2019
299594d
Merge branch 'master' into resource-aws-ec2-client-vpn-consolidation
gdavison Jun 23, 2020
69207fe
Authorization Rules do not require a Network Association
gdavison Jun 23, 2020
baaedf8
Formatting cleanup
gdavison Jun 23, 2020
d73e537
Authorization Rules are not required for Routes
gdavison Jun 23, 2020
cac47ca
Adds API resource parameter to `Exists` function
gdavison Jun 23, 2020
63f6b94
Removes deprecation of `aws_ec2_client_vpn_network_association` and r…
gdavison Jun 23, 2020
85d5e03
Moves Client VPN authorization rule to separate resource and adds bas…
gdavison Jun 24, 2020
0fede62
Adds waiters and `disappears` test
gdavison Jun 25, 2020
8300857
Handles multiple authorization groups
gdavison Jun 25, 2020
540f123
Removes `route` from `aws_ec2_client_vpn_endpoint.test`
gdavison Jun 25, 2020
9f7c429
Cleanup
gdavison Jun 25, 2020
3c41148
Adds import
gdavison Jun 25, 2020
47b626d
Adds correct filters to Destroy check
gdavison Jun 25, 2020
a617731
Adds tests for multiple subnets
gdavison Jun 25, 2020
b06cac4
Adds import documentation
gdavison Jun 26, 2020
c3e9794
Fixes documentation
gdavison Jun 26, 2020
fb0571d
Test cleanup and update to use Terraform 0.12 syntax
gdavison Jun 26, 2020
2256f43
Renames shared configuration functions
gdavison Jun 26, 2020
36185b3
Reorganizes shared test configurations
gdavison Jun 26, 2020
af65152
Consolidates retrieving individual Client VPN authorization rules
gdavison Jun 26, 2020
17426db
Adds semaphore to limit concurrency of Client VPN-related acceptance …
gdavison Jun 26, 2020
706c2d2
Removes unneeded Client VPN network association deletion from endpoin…
gdavison Jun 29, 2020
ebc3e37
Adds waiter for Client VPN to be deleted and removed
gdavison Jun 29, 2020
923ed2f
Moves Endpoint delete waiter to waiter package. Renames some error co…
gdavison Jun 30, 2020
aaf82b7
Combines Client VPN tests for synchronization
gdavison Jun 30, 2020
9c04ab5
Fixes test name
gdavison Jun 30, 2020
1b589c4
Merge branch 'master' into resource-aws-ec2-client-vpn-consolidation
gdavison Jun 30, 2020
269186a
Removes unused function
gdavison Jun 30, 2020
d85342a
Fixes synchronization for network association and consolidates test c…
gdavison Jul 1, 2020
a76229c
Moves waiter and status functions to `waiter` package
gdavison Jul 7, 2020
20569d6
Adds validity check for ID parsing
gdavison Jul 7, 2020
bf963ee
Moves synchronization functions to experimental package
gdavison Jul 7, 2020
bb1f70a
Fixes documentation re-ordering
gdavison Jul 7, 2020
3858262
Makes acceptance tests more stand-alone
gdavison Jul 7, 2020
0a56fc8
Update aws/internal/service/ec2/waiter/status.go
gdavison Jul 8, 2020
4403fca
Update aws/resource_aws_ec2_client_vpn_authorization_rule.go
gdavison Jul 8, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 2 additions & 24 deletions aws/ec2_filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ package aws

import (
"fmt"
"sort"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"

"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2"
)

// buildEC2AttributeFilterList takes a flat map of scalar attributes (most
Expand All @@ -33,29 +33,7 @@ import (
// the EC2 API, to aid in the implementation of Terraform data sources that
// retrieve data about EC2 objects.
func buildEC2AttributeFilterList(attrs map[string]string) []*ec2.Filter {
var filters []*ec2.Filter

// sort the filters by name to make the output deterministic
var names []string
for filterName := range attrs {
names = append(names, filterName)
}

sort.Strings(names)

for _, filterName := range names {
value := attrs[filterName]
if value == "" {
continue
}

filters = append(filters, &ec2.Filter{
Name: aws.String(filterName),
Values: []*string{aws.String(value)},
})
}

return filters
return tfec2.BuildAttributeFilterList(attrs)
}

// buildEC2TagFilterList takes a []*ec2.Tag and produces a []*ec2.Filter that
Expand Down
48 changes: 48 additions & 0 deletions aws/internal/experimental/sync/sync.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package sync

import (
"fmt"
"os"
"strconv"
"testing"
)

// Semaphore can be used to limit concurrent executions. This can be used to work with resources with low quotas
type Semaphore chan struct{}

// InitializeSemaphore initializes a semaphore with a default capacity or overrides it using an environment variable
// NOTE: this is currently an experimental feature and is likely to change. DO NOT USE.
func InitializeSemaphore(envvar string, defaultLimit int) Semaphore {
limit := defaultLimit
x := os.Getenv(envvar)
if x != "" {
var err error
limit, err = strconv.Atoi(x)
if err != nil {
panic(fmt.Errorf("could not parse %q: expected integer, got %q", envvar, x))
}
}
return make(Semaphore, limit)
}

// Wait waits for a semaphore before continuing
// NOTE: this is currently an experimental feature and is likely to change. DO NOT USE.
func (s Semaphore) Wait() {
s <- struct{}{}
}

// Notify releases a semaphore
// NOTE: this is currently an experimental feature and is likely to change. DO NOT USE.
func (s Semaphore) Notify() {
<-s
}

// TestAccPreCheckSyncronized waits for a semaphore and skips the test if there is no capacity
// NOTE: this is currently an experimental feature and is likely to change. DO NOT USE.
func TestAccPreCheckSyncronize(t *testing.T, semaphore Semaphore, resource string) {
if cap(semaphore) == 0 {
t.Skipf("concurrency for %s testing set to 0", resource)
}

semaphore.Wait()
}
24 changes: 24 additions & 0 deletions aws/internal/service/ec2/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package ec2

import (
"errors"

"github.com/aws/aws-sdk-go/aws/awserr"
)

// Copied from aws-sdk-go-base
// Can be removed when aws-sdk-go-base v0.6+ is merged
// TODO:
func ErrCodeEquals(err error, code string) bool {
var awsErr awserr.Error
if errors.As(err, &awsErr) {
return awsErr.Code() == code
}
return false
}

const ErrCodeClientVpnEndpointIdNotFound = "InvalidClientVpnEndpointId.NotFound"

const ErrCodeClientVpnAuthorizationRuleNotFound = "InvalidClientVpnEndpointAuthorizationRuleNotFound"

const ErrCodeClientVpnAssociationIdNotFound = "InvalidClientVpnAssociationId.NotFound"
gdavison marked this conversation as resolved.
Show resolved Hide resolved
55 changes: 55 additions & 0 deletions aws/internal/service/ec2/filter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package ec2

import (
"sort"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
)

// BuildAttributeFilterList takes a flat map of scalar attributes (most
// likely values extracted from a *schema.ResourceData on an EC2-querying
// data source) and produces a []*ec2.Filter representing an exact match
// for each of the given non-empty attributes.
//
// The keys of the given attributes map are the attribute names expected
// by the EC2 API, which are usually either in camelcase or with dash-separated
// words. We conventionally map these to underscore-separated identifiers
// with the same words when presenting these as data source query attributes
// in Terraform.
//
// It's the callers responsibility to transform any non-string values into
// the appropriate string serialization required by the AWS API when
// encoding the given filter. Any attributes given with empty string values
// are ignored, assuming that the user wishes to leave that attribute
// unconstrained while filtering.
//
// The purpose of this function is to create values to pass in
// for the "Filters" attribute on most of the "Describe..." API functions in
// the EC2 API, to aid in the implementation of Terraform data sources that
// retrieve data about EC2 objects.
func BuildAttributeFilterList(attrs map[string]string) []*ec2.Filter {
gdavison marked this conversation as resolved.
Show resolved Hide resolved
var filters []*ec2.Filter

// sort the filters by name to make the output deterministic
var names []string
for filterName := range attrs {
names = append(names, filterName)
}

sort.Strings(names)

for _, filterName := range names {
value := attrs[filterName]
if value == "" {
continue
}

filters = append(filters, &ec2.Filter{
Name: aws.String(filterName),
Values: []*string{aws.String(value)},
})
}

return filters
}
33 changes: 33 additions & 0 deletions aws/internal/service/ec2/finder/finder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package finder
gdavison marked this conversation as resolved.
Show resolved Hide resolved

import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2"
)

func ClientVpnAuthorizationRule(conn *ec2.EC2, endpointID, targetNetworkCidr, accessGroupID string) (*ec2.DescribeClientVpnAuthorizationRulesOutput, error) {
filters := map[string]string{
"destination-cidr": targetNetworkCidr,
}
if accessGroupID != "" {
filters["group-id"] = accessGroupID
}

input := &ec2.DescribeClientVpnAuthorizationRulesInput{
ClientVpnEndpointId: aws.String(endpointID),
Filters: tfec2.BuildAttributeFilterList(filters),
}

return conn.DescribeClientVpnAuthorizationRules(input)

}

func ClientVpnAuthorizationRuleByID(conn *ec2.EC2, authorizationRuleID string) (*ec2.DescribeClientVpnAuthorizationRulesOutput, error) {
endpointID, targetNetworkCidr, accessGroupID, err := tfec2.ClientVpnAuthorizationRuleParseID(authorizationRuleID)
if err != nil {
return nil, err
}

return ClientVpnAuthorizationRule(conn, endpointID, targetNetworkCidr, accessGroupID)
}
32 changes: 32 additions & 0 deletions aws/internal/service/ec2/id.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package ec2

import (
"fmt"
"strings"
)

const clientVpnAuthorizationRuleIDSeparator = ","

func ClientVpnAuthorizationRuleCreateID(endpointID, targetNetworkCidr, accessGroupID string) string {
parts := []string{endpointID, targetNetworkCidr}
if accessGroupID != "" {
parts = append(parts, accessGroupID)
}
id := strings.Join(parts, clientVpnAuthorizationRuleIDSeparator)
return id
}

func ClientVpnAuthorizationRuleParseID(id string) (string, string, string, error) {
parts := strings.Split(id, clientVpnAuthorizationRuleIDSeparator)
if len(parts) == 2 && parts[0] != "" && parts[1] != "" {
return parts[0], parts[1], "", nil
}
if len(parts) == 3 && parts[0] != "" && parts[1] != "" && parts[2] != "" {
return parts[0], parts[1], parts[2], nil
}

return "", "", "",
fmt.Errorf("unexpected format for ID (%q), expected endpoint-id"+clientVpnAuthorizationRuleIDSeparator+
"target-network-cidr or endpoint-id"+clientVpnAuthorizationRuleIDSeparator+"target-network-cidr"+
clientVpnAuthorizationRuleIDSeparator+"group-id", id)
}
76 changes: 76 additions & 0 deletions aws/internal/service/ec2/waiter/status.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package waiter

import (
"fmt"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder"
)

// LocalGatewayRouteTableVpcAssociationState fetches the LocalGatewayRouteTableVpcAssociation and its State
Expand Down Expand Up @@ -39,3 +43,75 @@ func LocalGatewayRouteTableVpcAssociationState(conn *ec2.EC2, localGatewayRouteT
return association, aws.StringValue(association.State), nil
}
}

const (
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the plan be to always separate const declarations in these files based on some criteria?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was following the model of a separate const block for each resource type as well as keeping them together with the corresponding Status function

Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely not something for this PR, but wonder if we should change these to per "resource" files then like status_ClientVpnEndpoint.go -- something to noodle over in #12840!

ClientVpnEndpointStatusNotFound = "NotFound"

ClientVpnEndpointStatusUnknown = "Unknown"
)

// ClientVpnEndpointStatus fetches the Client VPN endpoint and its Status
func ClientVpnEndpointStatus(conn *ec2.EC2, endpointID string) resource.StateRefreshFunc {
return func() (interface{}, string, error) {
result, err := conn.DescribeClientVpnEndpoints(&ec2.DescribeClientVpnEndpointsInput{
ClientVpnEndpointIds: aws.StringSlice([]string{endpointID}),
})
if tfec2.ErrCodeEquals(err, tfec2.ErrCodeClientVpnEndpointIdNotFound) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we'll need to globally decide whether errors like these should be handled in these service packages or downstream in the resource implementations so they can decide what to do with them. For example, this swallows available request ID information from the SDK unless debug logging is enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case, we need to handle at least some of the errors here, or else the calling WaitForState() will error out instead of returning cleanly

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the real difference is that if/when this code is generated, it then needs an additional layer of knowledge of errors rather than just how to handle the API structures. Not a big deal in the short term, but something to consider for longer term.

return nil, ClientVpnEndpointStatusNotFound, nil
}
if err != nil {
return nil, ClientVpnEndpointStatusUnknown, err
}

if result == nil || len(result.ClientVpnEndpoints) == 0 || result.ClientVpnEndpoints[0] == nil {
return nil, ClientVpnEndpointStatusNotFound, nil
}

endpoint := result.ClientVpnEndpoints[0]
if endpoint.Status == nil || endpoint.Status.Code == nil {
return endpoint, ClientVpnEndpointStatusUnknown, nil
}

return endpoint, aws.StringValue(endpoint.Status.Code), nil
}
}

const (
ClientVpnAuthorizationRuleStatusNotFound = "NotFound"

ClientVpnAuthorizationRuleStatusUnknown = "Unknown"
)

// ClientVpnAuthorizationRuleStatus fetches the Client VPN authorization rule and its Status
// TODO: This should be in the waiters package, but has a dependency on isAWSErr()
gdavison marked this conversation as resolved.
Show resolved Hide resolved
func ClientVpnAuthorizationRuleStatus(conn *ec2.EC2, authorizationRuleID string) resource.StateRefreshFunc {
return func() (interface{}, string, error) {
endpointID, targetNetworkCidr, accessGroupID, err := tfec2.ClientVpnAuthorizationRuleParseID(authorizationRuleID)
if err != nil {
return nil, ClientVpnAuthorizationRuleStatusUnknown, err
}

result, err := finder.ClientVpnAuthorizationRule(conn, endpointID, targetNetworkCidr, accessGroupID)
if tfec2.ErrCodeEquals(err, tfec2.ErrCodeClientVpnAuthorizationRuleNotFound) {
return nil, ClientVpnAuthorizationRuleStatusNotFound, nil
}
if err != nil {
return nil, ClientVpnAuthorizationRuleStatusUnknown, err
}

if result == nil || len(result.AuthorizationRules) == 0 || result.AuthorizationRules[0] == nil {
return nil, ClientVpnAuthorizationRuleStatusNotFound, nil
}

if len(result.AuthorizationRules) > 1 {
return nil, ClientVpnAuthorizationRuleStatusUnknown, fmt.Errorf("internal error: found %d results for Client VPN authorization rule (%s) status, need 1", len(result.AuthorizationRules), authorizationRuleID)
}

rule := result.AuthorizationRules[0]
if rule.Status == nil || rule.Status.Code == nil {
return rule, ClientVpnAuthorizationRuleStatusUnknown, nil
}

return rule, aws.StringValue(rule.Status.Code), nil
}
}
61 changes: 61 additions & 0 deletions aws/internal/service/ec2/waiter/waiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,64 @@ func LocalGatewayRouteTableVpcAssociationDisassociated(conn *ec2.EC2, localGatew

return nil, err
}

const (
ClientVpnEndpointDeletedTimout = 5 * time.Minute
)

func ClientVpnEndpointDeleted(conn *ec2.EC2, id string) (*ec2.ClientVpnEndpoint, error) {
stateConf := &resource.StateChangeConf{
Pending: []string{ec2.ClientVpnEndpointStatusCodeDeleting},
Target: []string{},
Refresh: ClientVpnEndpointStatus(conn, id),
Timeout: ClientVpnEndpointDeletedTimout,
}

outputRaw, err := stateConf.WaitForState()

if output, ok := outputRaw.(*ec2.ClientVpnEndpoint); ok {
return output, err
}

return nil, err
}

const (
ClientVpnAuthorizationRuleActiveTimeout = 1 * time.Minute

ClientVpnAuthorizationRuleRevokedTimeout = 1 * time.Minute
)

func ClientVpnAuthorizationRuleAuthorized(conn *ec2.EC2, authorizationRuleID string) (*ec2.AuthorizationRule, error) {
stateConf := &resource.StateChangeConf{
Pending: []string{ec2.ClientVpnAuthorizationRuleStatusCodeAuthorizing},
Target: []string{ec2.ClientVpnAuthorizationRuleStatusCodeActive},
Refresh: ClientVpnAuthorizationRuleStatus(conn, authorizationRuleID),
Timeout: ClientVpnAuthorizationRuleActiveTimeout,
}

outputRaw, err := stateConf.WaitForState()

if output, ok := outputRaw.(*ec2.AuthorizationRule); ok {
return output, err
}

return nil, err
}

func ClientVpnAuthorizationRuleRevoked(conn *ec2.EC2, authorizationRuleID string) (*ec2.AuthorizationRule, error) {
stateConf := &resource.StateChangeConf{
Pending: []string{ec2.ClientVpnAuthorizationRuleStatusCodeRevoking},
Target: []string{},
Refresh: ClientVpnAuthorizationRuleStatus(conn, authorizationRuleID),
Timeout: ClientVpnAuthorizationRuleRevokedTimeout,
}

outputRaw, err := stateConf.WaitForState()

if output, ok := outputRaw.(*ec2.AuthorizationRule); ok {
return output, err
}

return nil, err
}
Loading