-
Notifications
You must be signed in to change notification settings - Fork 367
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
upcoming: [M3-9088] - Add and update /v4/networking
endpoints and types for Linode Interfaces
#11559
base: develop
Are you sure you want to change the base?
Conversation
} | ||
|
||
export interface FirewallRules { | ||
fingerprint?: string; |
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 field and version
are mentioned in the documentation for the rules object when getting a firewall. I haven't seen them be returned when I tested the getTemplate
endpoints, so including them as optional fields
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.
Optional: Maybe eventually we should have a separate type for a firewall template's rule?
Not sure because the API spec claims same rules object as in Firewall GET
like you said. We could follow up on this later
vpc_nat_1_1?: { | ||
address: string; | ||
subnet_id: number; | ||
vpc_id: number; | ||
} | null; |
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.
adding this field (looks like we were missing it previously). It got returned either as the object or a null
value (or was omitted) when I tested the endpoint
*/ | ||
export const getTemplate = (templateSlug: string) => | ||
export const getTemplate = (templateSlug: FirewallTemplateSlug) => |
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.
These two endpoints (getTemplates
and getTemplate
) were added during the Securing VMs project (see internal ticket - I linked the spec there). I updated the templateSlug type to be more specific - I believe these are the three templates that exist so far, but am not sure. Since there may be a possibility for more slugs I'm not aware of, happy to switch back to string
too
I've currently typed it as
export type FirewallTemplateSlug = 'akamai-non-prod' | 'vpc' | 'public'
See internal comments on spec about some naming updates - when testing the endpoint, I saw that vpc
and public
are the returned slugs
entities: { | ||
id: number; | ||
type: FirewallDeviceEntityType; | ||
label: string; | ||
url: string; | ||
}[]; |
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.
replacing this with FirewallDeviceEntity since it already exists
export interface FirewallDeviceEntity {
id: number;
type: FirewallDeviceEntityType;
label: string;
url: string;
}
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.
Looks great! Thanks for those extra fixes in addition to the new types
} | ||
|
||
export interface FirewallRules { | ||
fingerprint?: string; |
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.
Optional: Maybe eventually we should have a separate type for a firewall template's rule?
Not sure because the API spec claims same rules object as in Firewall GET
like you said. We could follow up on this later
Co-authored-by: Banks Nussman <115251059+bnussman-akamai@users.noreply.github.com>
Coverage Report: ✅ |
Cloud Manager UI test results🎉 482 passing tests on test run #6 ↗︎
|
Description 📝
Updates existing and adds new /v4/networking endpoints for the Linode Interfaces project in accordance with API spec
Added a changeset for the Manager package after updating some factories. Is this changeset necessary? can remove it otherwise
How to test 🧪
Author Checklists
As an Author, to speed up the review process, I considered 🤔
👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support
As an Author, before moving this PR from Draft to Open, I confirmed ✅