Skip to content
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-9421] - Add API endpoints and types for /v4/nodebalancers #11832

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

harsh-akamai
Copy link
Contributor

Description 📝

Add new /v4/nodebalancers (& /v4beta/nodebalancers) endpoints for NB-VPC Integration

How to test 🧪

  • confirm changes match API spec
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


  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All unit tests are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@harsh-akamai harsh-akamai marked this pull request as ready for review March 18, 2025 10:42
@harsh-akamai harsh-akamai requested a review from a team as a code owner March 18, 2025 10:42
@harsh-akamai harsh-akamai requested review from bnussman-akamai and coliu-akamai and removed request for a team March 18, 2025 10:42
Copy link

Coverage Report:
Base Coverage: 80.07%
Current Coverage: 80.07%

nodes: array()
.of(nodeBalancerConfigNodeSchema)
.required()
.min(1, 'You must provide at least one back end node.'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we'll need a Validation changed changeset for this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this change since nodes are not a required field for updating configs, contrary to what the tech docs mention

@harsh-akamai harsh-akamai requested a review from a team as a code owner March 19, 2025 11:33
@harsh-akamai harsh-akamai requested review from dmcintyr-akamai and removed request for a team March 19, 2025 11:33
@harsh-akamai harsh-akamai force-pushed the M3-9241-add-endpoints-in-nb branch from 6e2aace to aa17b47 Compare March 19, 2025 11:34
@@ -235,7 +246,7 @@ export interface CreateNodeBalancerPayload {
configs: CreateNodeBalancerConfig[];
firewall_id?: number;
tags?: string[];
vpc?: {
vpcs?: {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback from previous PR comment

@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🎉 539 passing tests on test run #10 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing539 Passing3 Skipped101m 53s

Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, thank you!

*
* @param nodeBalancerId { number } The ID of the NodeBalancer to view vpc config info for.
*/
export const getNodeBalancerVPCsBeta = (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional: wondering if these should be renamed to getNodeBalancerVPCConfigsBeta since we're getting VPC config data, not VPCs specifically?

either makes sense to me though, especially since the endpoint itself uses /vpcs, not /vpc-configs.

@coliu-akamai coliu-akamai added the Approved Multiple approvals and ready to merge! label Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! NB-VPC Relating to NodeBalancer-VPC integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants