Skip to content

Conversation

@lgarber-akamai
Copy link
Contributor

@lgarber-akamai lgarber-akamai commented Mar 27, 2025

📝 Description

This pull request updates the linodego.Client creation logic to set the user-configured API token using the LinodeClient{}.SetToken(...) function rather than using a custom oauth2 transport. This allows the LINODE_CA environment variable to be specified without any additional logic to set the trusted root CA, which is necessary because linodego can only modify an HTTP client's trusted CAs if the user-provided client's RoundTripper is a *http.Transport.

✔️ How to Test

The following test steps assume you have pulled down this PR locally, have Ansible installed, have an SSH public key exported under ~/.ssh/id_rsa.pub, and have a valid Linode API token exported under the LINODE_TOKEN environment variable.

Integration Testing

  1. Run the following command to kick off the integration tests:
make int-test

Manual Testing

TODO

@lgarber-akamai lgarber-akamai added the bugfix for any bug fixes in the changelog. label Mar 27, 2025
@lgarber-akamai lgarber-akamai requested a review from a team as a code owner March 27, 2025 19:33
@lgarber-akamai lgarber-akamai requested review from yec-akamai and zliang-akamai and removed request for a team March 27, 2025 19:33
@lgarber-akamai lgarber-akamai changed the title fix: Configure API token using LinodeClient{}.SetToken(...) rather than transport; allow configuring LINODE_CA Configure API token using LinodeClient{}.SetToken(...) rather than transport; allow configuring LINODE_CA Mar 27, 2025
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.

The change looks good! However, we probably need to update the test case a bit. I ran the integration test but got current image used to create linode is not valid anymore: https://github.com/linode/docker-volume-linode/blob/main/integration-test/test.yaml#L38

I updated the image to use linode/ubuntu24.04 locally and it passed!

@lgarber-akamai
Copy link
Contributor Author

The change looks good! However, we probably need to update the test case a bit. I ran the integration test but got current image used to create linode is not valid anymore: https://github.com/linode/docker-volume-linode/blob/main/integration-test/test.yaml#L38

I updated the image to use linode/ubuntu24.04 locally and it passed!

Nice find! I can address that in this PR

Copy link
Member

@zliang-akamai zliang-akamai left a comment

Choose a reason for hiding this comment

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

Test works well, nice work!

@lgarber-akamai lgarber-akamai merged commit 756e051 into linode:main Apr 7, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix for any bug fixes in the changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants