feat(ecs): Implement load balancer views #8186
Conversation
|
The following commits need their title changed:
Please format your commit title into the form:
This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here. |
allisaurus
force-pushed
the
ecs-lbs
branch
2 times, most recently
from
4c0500d
to
b486dd7
Compare
| vpcName?: string; | ||
| } | ||
|
|
||
| export interface IALBListener { |
There was a problem hiding this comment.
Out of curiosity, what do we do for NetworkLoadBalancers/Listeners? Do we have an INLBListener?
There was a problem hiding this comment.
nice catch! right now we don't do anything differently, and I believe NLB listeners would contain a subset of the fields returned for ALBs (no rules, for example). Since ECS services could use either I'll rename this to be more general.
| userInfoEndpoint: string; | ||
| } | ||
|
|
||
| export interface IForwardConfig { |
There was a problem hiding this comment.
According to https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-elasticloadbalancingv2-listenerrule-forwardconfig.html, it looks like there's another property called TargetGroupStickinessConfig. Is that something we should add (whether we have the logic or not)?
There was a problem hiding this comment.
I was thinking of only adding properties we need to reference in the code, vs. everything that might exist. Though if it's confusing we can be exhaustive and add them.
There was a problem hiding this comment.
Makes sense. Should we also display these properties?
| clientSecret?: string; | ||
| idpLogoutUrl?: string; | ||
| issuer: string; | ||
| scope: string; |
There was a problem hiding this comment.
https://docs.aws.amazon.com/elasticloadbalancing/latest/APIReference/API_AuthenticateOidcActionConfig.html Scope is actually optional. Wouldn't this throw an exception if scope is not provided?
| idpLogoutUrl?: string; | ||
| issuer: string; | ||
| scope: string; | ||
| sessionCookieName: string; |
There was a problem hiding this comment.
https://docs.aws.amazon.com/elasticloadbalancing/latest/APIReference/API_AuthenticateOidcActionConfig.html sessionCookieName is also optional.
| type: IListenerActionType; | ||
| } | ||
|
|
||
| export interface IAuthenticateOidcActionConfig { |
There was a problem hiding this comment.
https://docs.aws.amazon.com/elasticloadbalancing/latest/APIReference/API_AuthenticateOidcActionConfig.html - There are 2 optional properties that are not specified here: UseExistingClientSecret and OnUnauthenticatedRequest. Are these properties that we want to expose?
| } | ||
|
|
||
| // see https://docs.aws.amazon.com/elasticloadbalancing/latest/APIReference/API_Action.html | ||
| export interface IListenerAction { |
There was a problem hiding this comment.
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-elasticloadbalancingv2-listener-defaultactions.html - Do we want to support cognito authentication(AuthenticateCognitoConfig)?
|
|
||
| public render(): React.ReactElement<EcsLoadBalancerClusterContainer> { | ||
| const { loadBalancer, showInstances, showServerGroups } = this.props; | ||
| const alb = loadBalancer as IEcsLoadBalancer; |
There was a problem hiding this comment.
Are we assuming that all loadbalancers will be ALBs?
There was a problem hiding this comment.
we shouldn't! will update
* feat(ecs): Implement load balancer views * accomodate NLB and optional fields Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Implements spinnaker/spinnaker#4797
Depends on: spinnaker/clouddriver#4526
Adds basic ECS load balancer and target group views to deck.
screenshots