fix(appmesh): Move Client Policy from Virtual Service to backend structure #12943
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
|
|
||
| import { CfnVirtualNode } from './appmesh.generated'; | ||
| import { ClientPolicy } from './client-policy'; | ||
| import { IVirtualService } from './virtual-service'; | ||
|
|
||
| // keep this import separate from other imports to reduce chance for merge conflicts with v2-main | ||
| // eslint-disable-next-line no-duplicate-imports, import/order | ||
| import { Construct } from '@aws-cdk/core'; | ||
|
|
||
| /** | ||
| * Represents the properties needed to define backend defaults | ||
| */ | ||
| export interface BackendDefaultsOptions { | ||
| /** | ||
| * Client policy for backend defaults | ||
| * | ||
| * @default none | ||
| */ | ||
| readonly clientPolicy?: ClientPolicy; | ||
| } | ||
|
|
||
| /** | ||
| * Represents the properties needed to define a backend | ||
| */ | ||
| export interface BackendOptions { | ||
| /** | ||
| * The Virtual Service this backend points to | ||
| */ | ||
| readonly virtualService: IVirtualService; | ||
| /** | ||
|
There was a problem hiding this comment. There should be a newline between comments and previous variable |
||
| * Client policy for a backend | ||
| * | ||
| * @default none | ||
| */ | ||
| readonly clientPolicy?: ClientPolicy; | ||
| } | ||
|
|
||
| /** | ||
| * Provides static factory methods to generate backend API structures | ||
| */ | ||
| export class Backends { | ||
| /** | ||
| * Creates a backend defaults | ||
| */ | ||
| public static backendDefaults(props: BackendDefaultsOptions): BackendDefaults { | ||
| return new BackendDefaults(props.clientPolicy); | ||
| } | ||
| /** | ||
| * Creates a named backend | ||
| */ | ||
| public static backend(props: BackendOptions): Backend { | ||
| return new Backend(props.virtualService, props.clientPolicy); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Represents all the backends that aren't specifically defined using the backend . | ||
| */ | ||
| export class BackendDefaults { | ||
|
There was a problem hiding this comment. I wonder if we could just make this an interface. I'm trying to think of a situation where we would need to enforce mutual exclusivity for this field and I'm not sure if we would need this. What do you think? There was a problem hiding this comment. Not sure if I'm following. Do you mean both There was a problem hiding this comment. Like, just defining BackendDefaults as an interface type interface BackendDefaults {
readonly clientPolicy?: ClientPolicy;
}and then instead of having to call new VirtualNode(stack, 'VirtualNode', {
...
backendDefaults: {
clientPolicy: appmesh.ClientPolicy.acm({ ... }),
}
);There was a problem hiding this comment. My understanding is the There was a problem hiding this comment. Yeah, I agree. It doesn't seem like the Maybe something like this is what we need? export interface BackendDefaults {
readonly clientPolicy?: ClientPolicy;
}
export interface Backend {
readonly clientPolicy?: ClientPolicy;
readonly virtualService: IVirtualService;
}There was a problem hiding this comment. I would hesitate to remove to remove the There was a problem hiding this comment. Fair enough. I defer to the expert(s) here 🙂. |
||
|
|
||
| constructor (private readonly clientPolicy: ClientPolicy | undefined) {} | ||
|
|
||
| /** | ||
| * Return backend defaults config | ||
| */ | ||
| public bind(_scope: Construct): CfnVirtualNode.BackendDefaultsProperty { | ||
| return { | ||
| clientPolicy: this.clientPolicy?.bind(_scope).clientPolicy, | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Represents the backend that a virtual node will send outbound traffic to | ||
| */ | ||
| export class Backend { | ||
|
|
||
| constructor (private readonly virtualService: IVirtualService, | ||
| private readonly clientPolicy: ClientPolicy | undefined) {} | ||
|
|
||
| /** | ||
| * Return backend config | ||
| */ | ||
| public bind(_scope: Construct): CfnVirtualNode.BackendProperty { | ||
| return { | ||
| virtualService: { | ||
| virtualServiceName: this.virtualService.virtualServiceName, | ||
| clientPolicy: this.clientPolicy?.bind(_scope).clientPolicy, | ||
| }, | ||
| }; | ||
| } | ||
| } | ||
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.
Remove this line
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 second this motion 🙂.