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

Support For Managing Reserved IP Addresses #579

Merged
merged 24 commits into from
Sep 17, 2024
Merged

Conversation

AniJ98
Copy link
Contributor

@AniJ98 AniJ98 commented Sep 9, 2024

📝 Description

This PR introduces comprehensive functionality and tests for Reserved IP Addresses in the Linodego client. The changes are necessary to provide robust support for managing Reserved IP addresses through the Linode API. The changes include:

Implementation of core Reserved IP operations:

  • Listing Reserved IPs
  • Getting a specific Reserved IP
  • Reserving a new IP
  • Deleting a Reserved IP

Test coverage:

  • TestReservedIPAddresses_InsufficientPermissions: Verifies behavior when the user lacks necessary permissions.
  • TestReservedIPAddresses_EndToEndTest: Performs a complete workflow test for users with proper permissions.
  • TestReservedIPAddresses_ListIPAddressesVariants: Tests various scenarios for listing IP addresses.
  • TestReservedIPAddresses_GetIPAddressVariants: Covers different cases for retrieving a specific IP address.
  • TestReservedIPAddresses_ReserveIPVariants: Tests multiple scenarios for reserving an IP, including edge cases.
  • TestReservedIPAddresses_DeleteIPAddressVariants: Verifies deletion functionality under various conditions.

✔️ How to Test

What are the steps to reproduce the issue or verify the changes?

To verify the changes related to Reserved IP functionality, follow these steps:

  1. Ensure you have a valid Linode API token with the "can_reserve_ip" permission enabled.
Set up your environment:


export LINODE_TOKEN="your_token_here"


Additionally you need to set the following if you want to test it in a different environment:


export LINODE_URL="https://api.linode.com"
export LINODE_API_VERSION="v4beta"

Navigate to the test directory within the linodego project.
Update the LINODE_TOKEN in the Makefile:


LINODE_TOKEN="your_token_here"

  1. Run the tests using one of the following commands:


To run all integration tests:


make testint

To run only Reserved IP related tests:


go test -v ./integration -run TestReservedIPAddresses


To run a specific test:

go test -v ./integration -run TestReservedIPAddresses_EndToEndTest

  1. Verify the test output for any failures or unexpected behavior.

Note:

Ensure you have proper permissions and sufficient quota in your Linode account to perform Reserved IP operations. Some tests may create and delete resources, so use a testing environment if possible.

@AniJ98 AniJ98 requested a review from a team as a code owner September 9, 2024 19:31
@AniJ98 AniJ98 requested review from yec-akamai and ezilber-akamai and removed request for a team September 9, 2024 19:31
Copy link
Contributor

@yec-akamai yec-akamai left a comment

Choose a reason for hiding this comment

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

LGTM. Great test coverage! Just one small comment:

Comment on lines 298 to 338
// Verify that we can't reserve more IPs
_, exceedErr := client.ReserveIPAddress(context.Background(), ReserveIPOptions{Region: "us-east"})
if exceedErr == nil {
t.Errorf("Expected error when exceeding reservation limit, got nil")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The reservation limit seems can be expanded. I think we might want to remove this verification to avoid unexpected failure.

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 check and re-recorded the fixture for this test.

Copy link
Contributor

@ykim-akamai ykim-akamai Sep 16, 2024

Choose a reason for hiding this comment

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

I think it is still a good test coverage to have. I pushed a commit separating into a new test case.

Copy link
Contributor

Choose a reason for hiding this comment

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

The new test case works well, thank you @ykim-1!

Copy link
Contributor

@yec-akamai yec-akamai left a comment

Choose a reason for hiding this comment

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

Tested and works well on my end, thank you for the contribution!

@ykim-akamai ykim-akamai self-requested a review September 16, 2024 17:57
Copy link
Contributor

@ykim-akamai ykim-akamai left a comment

Choose a reason for hiding this comment

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

LGTM, test pass locally and thank you for the contribution!

AniJ98 and others added 22 commits September 16, 2024 16:04
…d, InsuffecientPermission, ReserveIP, GetReservedIP, getReservedIPs, DeleteReservedIPs
… feature. Removed for loop to reserve IPs till limit is reached.
* build(deps): bump golang.org/x/oauth2 from 0.22.0 to 0.23.0

Bumps [golang.org/x/oauth2](https://github.com/golang/oauth2) from 0.22.0 to 0.23.0.
- [Commits](golang/oauth2@v0.22.0...v0.23.0)

---
updated-dependencies:
- dependency-name: golang.org/x/oauth2
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Ran make tidy

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: ezilber-akamai <ezilber@akamai.com>
* build(deps): bump golang.org/x/text from 0.17.0 to 0.18.0

Bumps [golang.org/x/text](https://github.com/golang/text) from 0.17.0 to 0.18.0.
- [Release notes](https://github.com/golang/text/releases)
- [Commits](golang/text@v0.17.0...v0.18.0)

---
updated-dependencies:
- dependency-name: golang.org/x/text
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* make tidy

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ye Chen <yechen@akamai.com>
…ing endpoints (linode#573)

* Add LKE types endpoints

* Support base struct; add NB types endpoints

* Add volume types

* Add network transfer prices

* Add price and region price structs

* Revert IPv6 fixtures

* Add missing fixtures
Copy link
Contributor

@ezilber-akamai ezilber-akamai left a comment

Choose a reason for hiding this comment

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

Nice work! Tests are passing on my end. Thanks for the contribution!

@yec-akamai yec-akamai merged commit 108afdf into linode:main Sep 17, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants