equinixmetal-archive / packngo

[Deprecated] A Golang client for the Equinix Metal API. (Packet is now Equinix Metal)
https://deploy.equinix.com/labs/equinix-sdk-go/
Other
79 stars 53 forks source link

introduce PACKET_API_URL env variable #351

Open displague opened 1 year ago

displague commented 1 year ago

Just as PACKET_AUTH_TOKEN can be used today to set an authentication token, PACKET_API_URL is being added to allow developers and users to set custom API URL targets. Some applications may already provide overrides via configuration files and environment. This environment variable will provide flexibility where applications did offer this.

The existing default value has been preserved for NewClient, NewClientWithAuth, and NewClientWithBaseURL. The former can now take advantage of PACKET_API_URL.

Fixes #322

ctreatma commented 1 year ago

I'm not clear what we gain by adding this. We already support alternative base URLs as an argument when creating a new packngo client, and we're using that in both metal-cli and terraform-provider-equinix.

displague commented 1 year ago

Oh! Terraform picked this up in the move to the terraform-provider-equinix provider, borrowing from the EQUINIX_API_ENDPOINT environment and endpoint provider config options. Looks like I may have introduced that for Metal too 🙃. This means that we can see how Prism reacts when Terraform E2E tests run against the API spec (https://github.com/packethost/packngo/issues/322#issuecomment-1439302924).

Metal CLI appears to be using the NewClientWithBaseURL function, but the base URL is not configurable today. This PR wouldn't help metal-cli out, which was one of the intents. https://github.com/equinix/metal-cli/blob/a5e101858bce400e75c0eee011770e8c75dd35e7/cmd/cli.go#L46-L55 The better approach for Metal CLI will be to make the API endpoint configurable through metal configuration options. https://github.com/equinix/metal-cli/issues/169

With those two benefits knocked out, we would get external configurability in any other tool built against packngo with this change and using NewClient (and not the NewClientWithBaseURL function). These would be any client not providing a bespoke configurable base URL option.

Given the primary targets would not be addressed by this PR, I'd be willing to close this and the parent issue in favor of deferring to tool owners and metal-go alternatives.

The PR is mostly a cleanup/refactor of how the URL is passed between client constructors and tests with the addition of the environment variable that is consulted for an override.

If this PR stands, more other question to consider is PACKNGO_API_BASE_URL vs PACKET_API_URL. The latter fits the default auth token environment, and the former fits the debug option.

displague commented 1 year ago

I attempted to use this feature, not remembering that the PR was stalled.

$ PACKNGO_DEBUG=1 PACKET_API_URL=https://api.packet.net/ metal devices get
2023/04/24 10:23:25
=======[REQUEST]=============
GET /metal/v1/projects/.../devices?include=facility HTTP/1.1
Host: api.equinix.com

I was looking for a simple way to confirm the following packet-python issue: https://github.com/packethost/packet-python/issues/135

I say this as additional context for how this type of ENV override feature can be handy. For example, packet-python does not provide a way to override the API URL: https://github.com/packethost/packet-python/blob/master/packet/baseapi.py#L57-L71