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.

Sign up for GitHub

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

support aws china for managed dns #854

Merged
merged 1 commit into from
Feb 27, 2020
Merged

support aws china for managed dns #854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions config/crds/hive_v1_dnszone.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ spec:
description: CredentialsSecretRef contains a reference to a secret
that contains AWS credentials for CRUD operations
type: object
region:
description: Region is the AWS region to use for route53 operations.
This defaults to us-east-1. For AWS China, use cn-northwest-1.
type: string
type: object
gcp:
description: GCP specifies GCP-specific cloud configuration
Expand Down
4 changes: 4 additions & 0 deletions config/crds/hive_v1_hiveconfig.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ spec:
parent ManageDNSConfig object. Secret should have AWS keys
named 'aws_access_key_id' and 'aws_secret_access_key'.
type: object
region:
description: Region is the AWS region to use for route53 operations.
This defaults to us-east-1. For AWS China, use cn-northwest-1.
type: string
type: object
domains:
description: Domains is the list of domains that hive will be
Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/hive/v1/dnszone_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ type AWSDNSZoneSpec struct {
// to these tags,the DNS Zone controller will set a hive.openhsift.io/hostedzone tag
// identifying the HostedZone record that it belongs to.
AdditionalTags []AWSResourceTag `json:"additionalTags,omitempty"`

// Region is the AWS region to use for route53 operations.
// This defaults to us-east-1.
// For AWS China, use cn-northwest-1.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it is a hard requirement for this field to be 'cn-northwest-1' when wanting to interact with AWS China? It appears that putting in 'cn-north-1' would also result in using the alternative API endpoint (with the region overridden to use 'cn-northwest-1' for the created AWS client).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was an open question that I had that I have not gotten an answer for yet. I don't have the capability to really test out whether it must be cn-northwest-1, can be cn-northwest-1, or must be cn-north-1 when the cluster is being installed into cn-north-1. If I understood, @dofinn correctly, he indicated to me that it can be northwest-1

Copy link
Contributor

Choose a reason for hiding this comment

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

I can confirm that the client requires it to be set to cn-northwest-1. I'm just observing that if you put cn-north-1 in the DNSZone, the client gets created with a region set to cn-northwest-1 b/c the DNSZone region has the magic cn- prefix.

Copy link
Contributor

Choose a reason for hiding this comment

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

AWS China tends to have a default region endpoint depending on the service. Unfortunately, its best to figure this out using trial and error at the moment. aws --debug route53 --endpoint-url https://api.route53.cn list-hosted-zones is very helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, @joelddiaz, I am a bit confused. Are you suggesting that there are changes that need to be made to the code? The way I see the code working now is that the clusterdeployment controller always sets the region in the DNSZone to cn-northwest-1 if the region of the ClusterDeployment is a China region. If I understand your second comment correctly, that is what we want to happen since the region used must be cn-northwest-1 when making route53 requests. If a user creates their own DNSZone, then they can set the region to whatever they like and that will be used to make the route53 requests--for better or worse.

Copy link
Member

Choose a reason for hiding this comment

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

We should use "cn-northwest-1", just get confirmation from AWS China SA, this service is hosted in "cn-northwest-1" region, here is some test I did"

Failed when using cn-north-1:
aws route53  list-hosted-zones  --region cn-north-1 --endpoint-url https://route53.amazonaws.com.cn 
aws route53   list-tags-for-resources  --region cn-north-1 --endpoint-url https://route53.amazonaws.com.cn --resource-id <id> --resource-type hostedzone
Succeed with cn-northwest-1:
aws route53  list-hosted-zones  --region cn-northwest-1 --endpoint-url https://route53.amazonaws.com.cn --> succeed
aws route53   list-tags-for-resources  --region cn-northwest-1 --endpoint-url https://route53.amazonaws.com.cn --resource-id <id> --resource-type hostedzone

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sorry, I misread where the cn- prefix checking is going on. I see now that the clusterdeployment controller will put in cn-northwest-1 for any clusterDeployment with the cn- region.

// +optional
Region string `json:"region,omitempty"`
}

// AWSResourceTag represents a tag that is applied to an AWS cloud resource
Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/hive/v1/hiveconfig_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@ type ManageDNSAWSConfig struct {
// Secret should have AWS keys named 'aws_access_key_id' and 'aws_secret_access_key'.
// +optional
CredentialsSecretRef corev1.LocalObjectReference `json:"credentialsSecretRef,omitempty"`

// Region is the AWS region to use for route53 operations.
// This defaults to us-east-1.
// For AWS China, use cn-northwest-1.
// +optional
Region string `json:"region,omitempty"`
}

// ManageDNSGCPConfig contains GCP-specific info to manage a given domain.
Expand Down
31 changes: 24 additions & 7 deletions pkg/awsclient/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/credentials"
"github.com/aws/aws-sdk-go/aws/endpoints"
"github.com/aws/aws-sdk-go/aws/request"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/ec2"
Expand All @@ -27,6 +28,8 @@ import (
"github.com/aws/aws-sdk-go/service/route53/route53iface"
"github.com/aws/aws-sdk-go/service/s3"
"github.com/aws/aws-sdk-go/service/s3/s3iface"

"github.com/openshift/hive/pkg/constants"
)

const (
Expand All @@ -52,7 +55,7 @@ func init() {

// Client is a wrapper object for actual AWS SDK clients to allow for easier testing.
type Client interface {
//EC2
// EC2
DescribeAvailabilityZones(*ec2.DescribeAvailabilityZonesInput) (*ec2.DescribeAvailabilityZonesOutput, error)
DescribeImages(*ec2.DescribeImagesInput) (*ec2.DescribeImagesOutput, error)
DescribeVpcs(*ec2.DescribeVpcsInput) (*ec2.DescribeVpcsOutput, error)
Expand All @@ -62,10 +65,10 @@ type Client interface {
DescribeInstances(*ec2.DescribeInstancesInput) (*ec2.DescribeInstancesOutput, error)
TerminateInstances(*ec2.TerminateInstancesInput) (*ec2.TerminateInstancesOutput, error)

//ELB
// ELB
RegisterInstancesWithLoadBalancer(*elb.RegisterInstancesWithLoadBalancerInput) (*elb.RegisterInstancesWithLoadBalancerOutput, error)

//IAM
// IAM
CreateAccessKey(*iam.CreateAccessKeyInput) (*iam.CreateAccessKeyOutput, error)
CreateUser(*iam.CreateUserInput) (*iam.CreateUserOutput, error)
DeleteAccessKey(*iam.DeleteAccessKeyInput) (*iam.DeleteAccessKeyOutput, error)
Expand All @@ -76,15 +79,15 @@ type Client interface {
ListUserPolicies(*iam.ListUserPoliciesInput) (*iam.ListUserPoliciesOutput, error)
PutUserPolicy(*iam.PutUserPolicyInput) (*iam.PutUserPolicyOutput, error)

//S3
// S3
CreateBucket(*s3.CreateBucketInput) (*s3.CreateBucketOutput, error)
DeleteBucket(*s3.DeleteBucketInput) (*s3.DeleteBucketOutput, error)
ListBuckets(*s3.ListBucketsInput) (*s3.ListBucketsOutput, error)

//Custom
// Custom
GetS3API() s3iface.S3API

//Route53
// Route53
CreateHostedZone(input *route53.CreateHostedZoneInput) (*route53.CreateHostedZoneOutput, error)
GetHostedZone(*route53.GetHostedZoneInput) (*route53.GetHostedZoneOutput, error)
ListTagsForResource(*route53.ListTagsForResourceInput) (*route53.ListTagsForResourceOutput, error)
Expand Down Expand Up @@ -300,7 +303,10 @@ func NewClient(kubeClient client.Client, secretName, namespace, region string) (
//
// Pass a nil secret to load credentials from the standard AWS environment variables.
func NewClientFromSecret(secret *corev1.Secret, region string) (Client, error) {
awsConfig := &aws.Config{Region: aws.String(region)}
awsConfig := &aws.Config{
Region: aws.String(region),
EndpointResolver: endpoints.ResolverFunc(awsChinaEndpointResolver),
}

// Special case to not use a secret to gather credentials.
if secret != nil {
Expand Down Expand Up @@ -339,3 +345,14 @@ func NewClientFromSecret(secret *corev1.Secret, region string) (Client, error) {
tagClient: resourcegroupstaggingapi.New(s),
}, nil
}

func awsChinaEndpointResolver(service, region string, optFns ...func(*endpoints.Options)) (endpoints.ResolvedEndpoint, error) {
if service != route53.EndpointsID || region != constants.AWSChinaRoute53Region {
return endpoints.DefaultResolver().EndpointFor(service, region, optFns...)
}

return endpoints.ResolvedEndpoint{
URL: "https://route53.amazonaws.com.cn",
PartitionID: "aws-cn",
}, nil
}
9 changes: 9 additions & 0 deletions pkg/constants/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,15 @@ const (

// PasswordSecretKey is a key used to store a password inside of a secret containing username / password credentials
PasswordSecretKey = "password"

// AWSRoute53Region is the region to use for route53 operations.
AWSRoute53Region = "us-east-1"

// AWSChinaRoute53Region is the region to use for AWS China route53 operations.
AWSChinaRoute53Region = "cn-northwest-1"

// AWSChinaRegionPrefix is the prefix for regions in AWS China.
AWSChinaRegionPrefix = "cn-"
)

// GetMergedPullSecretName returns name for merged pull secret name per cluster deployment
Expand Down
6 changes: 6 additions & 0 deletions pkg/controller/clusterdeployment/clusterdeployment_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"reflect"
"sort"
"strings"
"time"

"github.com/pkg/errors"
Expand Down Expand Up @@ -1359,9 +1360,14 @@ func (r *ReconcileClusterDeployment) createManagedDNSZone(cd *hivev1.ClusterDepl
for k, v := range cd.Spec.Platform.AWS.UserTags {
additionalTags = append(additionalTags, hivev1.AWSResourceTag{Key: k, Value: v})
}
region := ""
if strings.HasPrefix(cd.Spec.Platform.AWS.Region, constants.AWSChinaRegionPrefix) {
region = constants.AWSChinaRoute53Region
}
dnsZone.Spec.AWS = &hivev1.AWSDNSZoneSpec{
CredentialsSecretRef: cd.Spec.Platform.AWS.CredentialsSecretRef,
AdditionalTags: additionalTags,
Region: region,
}
case cd.Spec.Platform.GCP != nil:
dnsZone.Spec.GCP = &hivev1.GCPDNSZoneSpec{
Expand Down
7 changes: 6 additions & 1 deletion pkg/controller/dnsendpoint/dnsendpoint_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/source"

hivev1 "github.com/openshift/hive/pkg/apis/hive/v1"
"github.com/openshift/hive/pkg/constants"
"github.com/openshift/hive/pkg/controller/dnsendpoint/nameserver"
hivemetrics "github.com/openshift/hive/pkg/controller/metrics"
controllerutils "github.com/openshift/hive/pkg/controller/utils"
Expand Down Expand Up @@ -277,7 +278,11 @@ func createNameServerQuery(c client.Client, logger log.FieldLogger, managedDomai
if managedDomain.AWS != nil {
secretName := managedDomain.AWS.CredentialsSecretRef.Name
logger.Infof("using aws creds for managed domains stored in %q secret", secretName)
return nameserver.NewAWSQuery(c, secretName)
region := managedDomain.AWS.Region
if region == "" {
region = constants.AWSRoute53Region
}
return nameserver.NewAWSQuery(c, secretName, region)
}
if managedDomain.GCP != nil {
secretName := managedDomain.GCP.CredentialsSecretRef.Name
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/dnsendpoint/nameserver/aws.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ import (
)

// NewAWSQuery creates a new name server query for AWS.
func NewAWSQuery(c client.Client, credsSecretName string) Query {
func NewAWSQuery(c client.Client, credsSecretName string, region string) Query {
return &awsQuery{
getAWSClient: func() (awsclient.Client, error) {
awsClient, err := awsclient.NewClient(c, credsSecretName, constants.HiveNamespace, "us-east-1")
awsClient, err := awsclient.NewClient(c, credsSecretName, constants.HiveNamespace, region)
return awsClient, errors.Wrap(err, "error creating AWS client")
},
}
Expand Down
12 changes: 7 additions & 5 deletions pkg/controller/dnszone/awsactuator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ import (

hivev1 "github.com/openshift/hive/pkg/apis/hive/v1"
awsclient "github.com/openshift/hive/pkg/awsclient"
"github.com/openshift/hive/pkg/constants"
)

const (
hiveDNSZoneAWSTag = "hive.openshift.io/dnszone"
defaultRegionEndpoint = "us-east-1"
hiveDNSZoneAWSTag = "hive.openshift.io/dnszone"
)

// Ensure AWSActuator implements the Actuator interface. This will fail at compile time when false.
Expand Down Expand Up @@ -54,9 +54,11 @@ func NewAWSActuator(
dnsZone *hivev1.DNSZone,
awsClientBuilder awsClientBuilderType,
) (*AWSActuator, error) {
// Route53 is a regionless service, we specify a default region just for the purpose of creating
// our client configuration.
awsClient, err := awsClientBuilder(secret, defaultRegionEndpoint)
region := dnsZone.Spec.AWS.Region
if region == "" {
region = constants.AWSRoute53Region
}
awsClient, err := awsClientBuilder(secret, region)
if err != nil {
logger.WithError(err).Error("Error creating AWSClient")
return nil, err
Expand Down
8 changes: 8 additions & 0 deletions pkg/operator/assets/bindata.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.