-
Notifications
You must be signed in to change notification settings - Fork 3.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
fix: (aws-ec2): fix vpc endpoint incorrect issue in China region #16139
Changes from 3 commits
c796fc6
66e623b
e2e20a4
c191933
9aded26
a27acc1
4fac703
63f167a
fe93aa8
b37b4c9
f606f18
138c50f
88999ab
20f6008
105846d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -326,9 +326,32 @@ export class InterfaceVpcEndpointAwsService implements IInterfaceVpcEndpointServ | |||||
const region = Lazy.uncachedString({ | ||||||
produce: (context) => Stack.of(context.scope).region, | ||||||
}); | ||||||
this.name = `${prefix || 'com.amazonaws'}.${region}.${name}`; | ||||||
const regionPrefix =Lazy.uncachedString({ | ||||||
produce: (context) => { | ||||||
const regionName = Stack.of(context.scope).region; | ||||||
return this.getDefaultEndpointPrefix(name, regionName); | ||||||
}, | ||||||
}); | ||||||
|
||||||
this.name = `${prefix || regionPrefix}.${region}.${name}`; | ||||||
this.port = port || 443; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Get the endpoint prefix for the service in the specified region | ||||||
* because the prefix for some of the services in cn-north-1 and cn-northwest-1 are different | ||||||
*/ | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can add an AWS CLI sample in the doc comments showing how to get all endpoints from a region. That would be helpful. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, I'll add the AWS CLI sample to the comments |
||||||
private getDefaultEndpointPrefix(name: string, region: string) { | ||||||
const VPC_ENDPOINT_SERVICE_EXCEPTIONS: { [region: string]: string[] } = { | ||||||
'cn-north-1': ['application-autoscaling', 'athena', 'autoscaling', 'awsconnector', 'cassandra', 'cloudformation', 'codedeploy-commands-secure', 'databrew', 'dms', 'ebs', 'ec2', 'ecr.api', 'ecr.dkr', 'elasticbeanstalk', 'elasticfilesystem', 'elasticfilesystem-fips', 'execute-api', 'imagebuilder', 'iotsitewise.api', 'iotsitewise.data', 'kinesis-streams', 'lambda', 'license-manager', 'monitoring', 'rds', 'redshift', 'redshift-data', 's3', 'sagemaker.api', 'sagemaker.featurestore-runtime', 'sagemaker.runtime', 'servicecatalog', 'sms', 'sqs', 'states', 'sts', 'synthetics', 'transcribe.cn', 'transcribestreaming', 'transfer', 'xray'], | ||||||
'cn-northwest-1': ['application-autoscaling', 'athena', 'autoscaling', 'awsconnector', 'cassandra', 'cloudformation', 'codedeploy-commands-secure', 'databrew', 'dms', 'ebs', 'ec2', 'ecr.api', 'ecr.dkr', 'elasticbeanstalk', 'elasticfilesystem', 'elasticfilesystem-fips', 'execute-api', 'imagebuilder', 'kinesis-streams', 'lambda', 'license-manager', 'monitoring', 'rds', 'redshift', 'redshift-data', 's3', 'sagemaker.api', 'sagemaker.featurestore-runtime', 'sagemaker.runtime', 'servicecatalog', 'sms', 'sqs', 'states', 'sts', 'synthetics', 'transcribe.cn', 'transcribestreaming', 'transfer', 'workspaces', 'xray'], | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's wrap the lines to make it more readable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good suggestions, thanks Pahud |
||||||
}; | ||||||
if (VPC_ENDPOINT_SERVICE_EXCEPTIONS.hasOwnProperty(region) && VPC_ENDPOINT_SERVICE_EXCEPTIONS[region].includes(name)) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I believe this should be working. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It will be more concise, thanks Pahud for your suggestion, I'll update it |
||||||
return 'cn.com.amazonaws'; | ||||||
} else { | ||||||
return 'com.amazonaws'; | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
/** | ||||||
|
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.
How about just call it
defaultEndpointPrefix
?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.
Thanks @pahud, that's a good suggestion, I'll update it.