-
Notifications
You must be signed in to change notification settings - Fork 38
chore: Updates Project CFN resource handler to accept Lambda Proxy ARN as input & use as Http transport in Atlas SDK client #1300
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
base: master
Are you sure you want to change the base?
Conversation
@@ -127,6 +127,10 @@ | |||
"description": "Profile used to provide credentials information, (a secret with the cfn/atlas/profile/{Profile}, is required), if not provided default is used", | |||
"default": "default" | |||
}, | |||
"LambdaProxyArn": { |
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.
Alternatively, LambdaArn may be taken as an input in the Profile
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 would definitely have it in the Profile so we don't need to change every single resource
} | ||
|
||
// This method currently uses extensive logging for POC purpose which should be reduced | ||
func newAtlasV2ClientWithLambdaProxySupport(req *handler.Request, profileName *string, profileNamePrefixRequired bool, lambdaArn *string) (*MongoDBClient, *handler.ProgressEvent) { |
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.
Temporarily added this new method for the POC but this logic could simply be added to existing newAtlasV2Client() method
lambdaTransport := newLambdaForwardingTransport(req, *lambdaArn) | ||
digestTransport := digest.NewTransport(prof.PublicKey, prof.PrivateKey) | ||
// Set the underlying transport to our Lambda transport | ||
digestTransport.Transport = lambdaTransport |
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.
alternatively can use NewTransportWithHTTPRoundTripper()
c := Config{BaseURL: prof.BaseURL, DebugClient: prof.UseDebug()} | ||
log.Printf("Config initialized: %+v", c) | ||
|
||
sdk20231115002Client, err := c.NewSDKv20231115002Client(client) |
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.
if I read this correctly.. in the CFN implementation we are still going use the SDK. The SDK is instantiated using a different client that passes through the lambda invocation. Am I right?
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.
Right.
raise ValueError("EC2_PROXY_ENDPOINT is not set") | ||
logger.debug(f"EC2_PROXY_ENDPOINT: {EC2_PROXY_ENDPOINT}") | ||
|
||
def lambda_handler(event, context): |
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.
this is how customer should define the lambda function in their own AWS Account. right?
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.
yes
|
||
var client *http.Client | ||
if lambdaArn != nil && *lambdaArn != "" { | ||
log.Printf("Using chained digest transport with Lambda forwarding. Lambda ARN: %s", *lambdaArn) |
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.
Cool!
This PR has gone 30 days without any activity and meets the project’s definition of "stale". This will be auto-closed if there is no new activity over the next 30 days. If the issue is still relevant and active, you can simply comment with a "bump" to keep it open, or add the label "not_stale". Thanks for keeping our repository healthy! |
Proposed changes
Jira ticket: CLOUDP-303001
Please include a summary of the fix/feature/change, including any relevant motivation and context.
Link to any related issue(s):
Type of change:
expected)
Manual QA performed:
Required Checklist:
make fmt
and formatted my codeworks in Atlas
Further comments