Skip to content

Commit

Permalink
fix: [M3-7824] - ACLB TCP Rule Creation and other fixes (linode#10264)
Browse files Browse the repository at this point in the history 
* don't send `match_condition` for TCP rules and other fixes

* changesets and validation change

* clean up validation schemas

---------

Co-authored-by: Banks Nussman <banks@nussman.us>
  • Loading branch information
@bnussman-akamai @bnussman
bnussman-akamai and bnussman authored Mar 7, 2024
1 parent 491c106 commit 6127288
Show file tree
Hide file tree
Showing 10 changed files with 122 additions and 94 deletions.
5 changes: 5 additions & 0 deletions packages/api-v4/.changeset/pr-10264-changed-1709750042330.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/api-v4": Changed
---

Made `match_condition` optional in Rule types to support TCP rules ([#10264](https://github.com/linode/manager/pull/10264))
6 changes: 3 additions & 3 deletions packages/api-v4/src/aclb/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export interface Route {
label: string;
protocol: RouteProtocol;
rules: {
match_condition: MatchCondition;
match_condition?: MatchCondition;
service_targets: {
id: number;
label: string;
Expand All @@ -83,7 +83,7 @@ export interface CreateRoutePayload {
}

export interface Rule {
match_condition: MatchCondition;
match_condition?: MatchCondition;
service_targets: {
id: number;
label: string;
Expand All @@ -92,7 +92,7 @@ export interface Rule {
}

export interface RulePayload {
match_condition: MatchCondition;
match_condition?: MatchCondition;
service_targets: {
id: number;
label: string;
Expand Down
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10264-fixed-1709749998087.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Fixed
---

ACLB TCP rule creation ([#10264](https://github.com/linode/manager/pull/10264))
8 changes: 4 additions & 4 deletions packages/manager/cypress/e2e/core/loadBalancers/load-balancer-routes.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ describe('Akamai Cloud Load Balancer routes page', () => {
.should('be.visible')
.within(() => {
cy.findByLabelText('Hostname Match (optional)')
.should('have.value', routes[0].rules[0].match_condition.hostname)
.should('have.value', routes[0].rules[0].match_condition!.hostname)
.clear()
.type('example.com');

Expand All @@ -558,7 +558,7 @@ describe('Akamai Cloud Load Balancer routes page', () => {
cy.findByLabelText('Match Value')
.should(
'have.value',
routes[0].rules[0].match_condition.match_value
routes[0].rules[0].match_condition!.match_value
)
.clear()
.type('x-header=my-header-value');
Expand Down Expand Up @@ -604,7 +604,7 @@ describe('Akamai Cloud Load Balancer routes page', () => {

// Verify all rules are shown
for (const rule of routes[0].rules) {
cy.findByText(rule.match_condition.match_value).should('be.visible');
cy.findByText(rule.match_condition!.match_value).should('be.visible');
}

const indexOfRuleToDelete = 1;
Expand All @@ -625,7 +625,7 @@ describe('Akamai Cloud Load Balancer routes page', () => {

// Verify the deleted rule no longer shows
cy.findByText(
routes[0].rules[indexOfRuleToDelete].match_condition.match_value
routes[0].rules[indexOfRuleToDelete].match_condition!.match_value
).should('not.exist');
});
});
Expand Down
8 changes: 4 additions & 4 deletions packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/RuleDrawer.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,25 +54,25 @@ describe('RuleDrawer', () => {

const hostnameField = getByLabelText('Hostname Match (optional)');
expect(hostnameField).toHaveDisplayValue(
props.route.rules[0].match_condition.hostname!
props.route.rules[0].match_condition!.hostname!
);

const matchTypeField = getByLabelText('Match Type');
expect(matchTypeField).toHaveDisplayValue('Path Prefix');

const matchValueField = getByLabelText('Match Value');
expect(matchValueField).toHaveDisplayValue(
props.route.rules[0].match_condition.match_value
props.route.rules[0].match_condition!.match_value
);

const cookieField = getByLabelText('Cookie Key');
expect(cookieField).toHaveDisplayValue(
props.route.rules[0].match_condition.session_stickiness_cookie!
props.route.rules[0].match_condition!.session_stickiness_cookie!
);

const ttlField = getByLabelText('Stickiness TTL');
expect(ttlField).toHaveDisplayValue(
String(props.route.rules[0].match_condition.session_stickiness_ttl)
String(props.route.rules[0].match_condition!.session_stickiness_ttl)
);
});
});
74 changes: 42 additions & 32 deletions packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/RuleDrawer.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,16 @@ import { useLoadBalancerRouteUpdateMutation } from 'src/queries/aclb/routes';
import { getFormikErrorsFromAPIErrors } from 'src/utilities/formikErrorUtils';

import { ServiceTargetSelect } from '../ServiceTargets/ServiceTargetSelect';
import { MatchTypeInfo } from './MatchTypeInfo';
import { ROUTE_COPY } from './constants';
import { MatchTypeInfo } from './MatchTypeInfo';
import {
TimeUnit,
defaultServiceTarget,
defaultTTL,
defaultTTLUnit,
getInitialValues,
getIsSessionStickinessEnabled,
getNormalizedRulePayload,
initialValues,
matchTypeOptions,
matchValuePlaceholder,
stickyOptions,
Expand Down Expand Up @@ -77,6 +77,8 @@ export const RuleDrawer = (props: Props) => {

const [ttlUnit, setTTLUnit] = useState<TimeUnit>(defaultTTLUnit);

const initialValues = getInitialValues(protocol);

const formik = useFormik<RulePayload>({
enableReinitialize: true,
initialValues: isEditMode
Expand Down Expand Up @@ -155,7 +157,7 @@ export const RuleDrawer = (props: Props) => {
const isStickinessEnabled = getIsSessionStickinessEnabled(formik.values);

const cookieType =
formik.values.match_condition.session_stickiness_ttl === null
formik.values.match_condition?.session_stickiness_ttl === null
? stickyOptions[1]
: stickyOptions[0];

Expand Down Expand Up @@ -219,27 +221,22 @@ export const RuleDrawer = (props: Props) => {
{route?.protocol !== 'tcp' && (
<>
<TextField
errorText={
formik.touched.match_condition?.hostname
? formik.errors.match_condition?.hostname
: undefined
}
errorText={getIn(formik.errors, 'match_condition.hostname')}
label="Hostname Match"
labelTooltipText={ROUTE_COPY.Rule.Hostname}
name="match_condition.hostname"
onBlur={formik.handleBlur}
onChange={formik.handleChange}
optional
placeholder="www.example.org"
value={formik.values.match_condition.hostname}
value={formik.values.match_condition?.hostname}
/>
<Stack direction="row" spacing={2}>
<Autocomplete
errorText={
formik.touched.match_condition?.match_field
? formik.errors.match_condition?.match_field
: undefined
}
errorText={getIn(
formik.errors,
'match_condition.match_field'
)}
onBlur={() =>
formik.setFieldTouched('match_condition.match_field')
}
Expand All @@ -253,7 +250,7 @@ export const RuleDrawer = (props: Props) => {
matchTypeOptions.find(
(option) =>
option.value ===
formik.values.match_condition.match_field
formik.values.match_condition?.match_field
) ?? matchTypeOptions[0]
}
disableClearable
Expand All @@ -264,26 +261,28 @@ export const RuleDrawer = (props: Props) => {
/>
<TextField
errorText={
formik.touched.match_condition?.match_value
? formik.errors.match_condition?.match_value
getIn(formik.touched, 'match_condition.match_value')
? getIn(formik.errors, 'match_condition.match_value')
: undefined
}
labelTooltipText={
ROUTE_COPY.Rule.MatchValue[
formik.values.match_condition.match_field
formik.values.match_condition?.match_field ??
'always_match'
]
}
placeholder={
matchValuePlaceholder[
formik.values.match_condition.match_field
formik.values.match_condition?.match_field ??
'always_match'
]
}
containerProps={{ sx: { flexGrow: 1 } }}
label="Match Value"
name="match_condition.match_value"
onBlur={formik.handleBlur}
onChange={formik.handleChange}
value={formik.values.match_condition.match_value}
value={formik.values.match_condition?.match_value}
/>
</Stack>
<Stack alignItems="center" direction="row" gap={2}>
Expand Down Expand Up @@ -419,14 +418,19 @@ export const RuleDrawer = (props: Props) => {
/>
<TextField
errorText={
formik.touched.match_condition?.session_stickiness_cookie
? formik.errors.match_condition
?.session_stickiness_cookie
getIn(
formik.touched,
'match_condition.session_stickiness_cookie'
)
? getIn(
formik.errors,
'match_condition.session_stickiness_cookie'
)
: undefined
}
value={
formik.values.match_condition.session_stickiness_cookie ??
''
formik.values.match_condition
?.session_stickiness_cookie ?? ''
}
label="Cookie Key"
labelTooltipText={ROUTE_COPY.Rule.Stickiness.Cookie}
Expand All @@ -439,9 +443,14 @@ export const RuleDrawer = (props: Props) => {
<Stack direction="row" spacing={1}>
<TextField
errorText={
formik.touched.match_condition?.session_stickiness_ttl
? formik.errors.match_condition
?.session_stickiness_ttl
getIn(
formik.touched,
'match_condition.session_stickiness_ttl'
)
? getIn(
formik.errors,
'match_condition.session_stickiness_ttl'
)
: undefined
}
onChange={(e) =>
Expand All @@ -453,7 +462,7 @@ export const RuleDrawer = (props: Props) => {
}
value={
(formik.values.match_condition
.session_stickiness_ttl ?? 0) /
?.session_stickiness_ttl ?? 0) /
timeUnitFactorMap[ttlUnit]
}
label="Stickiness TTL"
Expand All @@ -471,11 +480,12 @@ export const RuleDrawer = (props: Props) => {
setTTLUnit(option.key);

if (
formik.values.match_condition.session_stickiness_ttl
formik.values.match_condition
?.session_stickiness_ttl
) {
const oldValue =
formik.values.match_condition
.session_stickiness_ttl;
?.session_stickiness_ttl;

formik.setFieldValue(
'match_condition.session_stickiness_ttl',
Expand All @@ -489,7 +499,7 @@ export const RuleDrawer = (props: Props) => {
disableClearable
label="test"
options={timeUnitOptions}
sx={{ marginTop: '45px !important', minWidth: '140px' }}
sx={{ marginTop: '52px !important', minWidth: '140px' }}
textFieldProps={{ hideLabel: true }}
/>
</Stack>
Expand Down
41 changes: 24 additions & 17 deletions packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,28 @@ export const defaultServiceTarget = {

export const defaultTTLUnit = 'second';

export const initialValues = {
match_condition: {
hostname: '',
match_field: 'path_prefix' as const,
match_value: '',
session_stickiness_cookie: null,
session_stickiness_ttl: null,
},
service_targets: [defaultServiceTarget],
export const getInitialValues = (protocol: Route['protocol']) => {
if (protocol === 'tcp') {
return { service_targets: [defaultServiceTarget] };
}
return {
match_condition: {
hostname: '',
match_field: 'path_prefix' as const,
match_value: '',
session_stickiness_cookie: null,
session_stickiness_ttl: null,
},
service_targets: [defaultServiceTarget],
};
};

export const getIsSessionStickinessEnabled = (
rule: Rule | RulePayload | RuleCreatePayload
) => {
return (
rule.match_condition.session_stickiness_cookie !== null ||
rule.match_condition.session_stickiness_ttl !== null
rule.match_condition?.session_stickiness_cookie !== null ||
rule.match_condition?.session_stickiness_ttl !== null
);
};

Expand All @@ -76,12 +81,14 @@ export const getIsSessionStickinessEnabled = (
* so that the API accepts the payload.
*/
export const getNormalizedRulePayload = (rule: RulePayload) => ({
match_condition: {
...rule.match_condition,
hostname: rule.match_condition.hostname
? rule.match_condition.hostname
: null,
},
match_condition: rule.match_condition
? {
...rule.match_condition,
hostname: rule.match_condition.hostname
? rule.match_condition.hostname
: null,
}
: undefined,
service_targets: rule.service_targets,
});

Expand Down
Loading

0 comments on commit 6127288

Please sign in to comment.