refactor(rt)!: implement kmp ecs provider #475
Conversation
|
A new generated diff is ready to view: __generated-main...__generated-feat-ecs-provider |
|
|
||
| public constructor() : this(Platform, CrtHttpEngine()) | ||
|
|
||
| private val retryMiddleware = run { |
|
|
||
| if (url.scheme == Protocol.HTTPS) return url | ||
|
|
||
| // TODO - validate loopback via DNS resolution instead of fixed set. Custom host names (including localhost) that |
There was a problem hiding this comment.
question
will this require some KMP work?
There was a problem hiding this comment.
Yeah that's why it isn't implemented. We need the ability to actually resolve a host to something like List<InetAddress> and verify that each one is in fact the loopback device (e.g. InetAddres.isLoopBackDevice(): Boolean).
ECS itself only uses the relative URI, this only comes into play when the full URI is set (and not using https). I believe greengrass is the primary user of this for something but I don't recall what. Seems safe to wait until it's needed. I created #476 to track it.
| internal class EcsCredentialsRetryPolicy : RetryPolicy<Any?> { | ||
| override fun evaluate(result: Result<Any?>): RetryDirective = when { | ||
| result.isSuccess -> RetryDirective.TerminateAndSucceed | ||
| else -> evaluate(result.exceptionOrNull()!!) |
There was a problem hiding this comment.
nit
it seems a little weird to call an ...orNull() function and then !! it. Is there a reason this is needed?
There was a problem hiding this comment.
Ya I agree it's a little weird but there is no function on Result that provides the exception or throws. The success case has functions like getOrThrow() but nothing like that for the error case. No smart casting to be had either :(
Seems like something is missing from Result to me but this should be safe since we know it's the error case.
| @@ -0,0 +1,70 @@ | |||
| # CDK stack for ECS credentials provider testing | |||
| @@ -0,0 +1,34 @@ | |||
| plugins { | |||
There was a problem hiding this comment.
nit
pull the version strings out property references (log4j, coroutines)
| import { EcsCredentialsStack } from '../lib/ecs-credentials-stack'; | ||
|
|
||
| const app = new cdk.App(); | ||
| new EcsCredentialsStack(app, 'EcsCredentialsStack', { |
There was a problem hiding this comment.
question
does this reference any cloud things that may change over time that we may need to manage? I'm not familiar with CDK myself.
There was a problem hiding this comment.
CDK is just a wrapper around cloudformation templates. To answer your question though no there is nothing to manage here. The expected usage of this is for one off testing. Developers would deploy, run whatever tests they need, and tear down the stack when they are done.
|
|
||
| const taskDefinition = new ecs.FargateTaskDefinition(this, 'TaskDef', { | ||
| memoryLimitMiB: 512, | ||
| cpu: 256, |
There was a problem hiding this comment.
Yeah it's a bit of an odd one but it's correct. 256 maps to .25 vCPU
See docs here
| @@ -0,0 +1,17 @@ | |||
| // import * as cdk from 'aws-cdk-lib'; | |||
There was a problem hiding this comment.
nit
is most of this commented out for a reason?
There was a problem hiding this comment.
This can be deleted, it's part of cdk init scaffolding 🤷
|
A new generated diff is ready to view: __generated-main...__generated-feat-ecs-provider |
|
SonarCloud Quality Gate failed.
|
|
A new generated diff is ready to view: __generated-main...__generated-feat-ecs-provider |
…ible (#469) Refactor credential providers to remove CRT dependency and make them KMP compatible. Added SSO provider to default chain. Lots of misc cleanup and improvements. * feat(rt): standalone sso credentials provider (#462) * refactor(rt)!: generated sts and sts web identity credential providers (#470) * refactor(rt)!: implement kmp ecs provider (#475) * feat(rt)!: implement kmp profile credentials provider (#478) * feat(rt)!: kmp default credentials provider chain (#491) * fix: work around machine-specific Gradle bug with aws-config variants (#496) * fix: credentials provider ownership (#498) Co-authored-by: Ian Botsford <83236726+ianbotsf@users.noreply.github.com>
2 participants
Issue #
N/A
Description of changes
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.