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

feat: add provider specific proxy option #1237

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sboschman
Copy link
Contributor

Issue: #982

Depends on pr rancher/norman #453

As this pr depends on norman it is a draft pr, till we can get the required change merged into norman.

Problem

Currently a proxy url can only be set as environment variable, which is used by all providers in the terraform run. If only Rancher requests should be proxied, a provider based proxy setting is required.

Solution

Added extra provider config option proxy_url to set a proxy server to use to connect to the Rancher instance.

Testing

Engineering Testing

Manual Testing

Locally tested to run a tf plan/apply with a Rancher instance behind a Cloudflare Zero Trust tunnel, requiring a localhost proxy to connect and authenticate to Cloudflare.

Automated Testing

Added testcases to check the validation of the proxy_url config. It should only accept parsable http, https or socks5 urls.

QA Testing Considerations

Regressions Considerations

@kkaempf
Copy link
Contributor

kkaempf commented Dec 8, 2023

closing stale PRs.
If you still intend to work on this, please reopen.

@kkaempf kkaempf closed this Dec 8, 2023
@sboschman
Copy link
Contributor Author

I am waiting for rancher/norman#453 to be merged, this draft PR is working (we are running with a self compiled provider binary based on this patch as local override).

@kkaempf , do you have any idea who we could ping with regards to that rancher/norman PR to get this moving along?

@ericpromislow
Copy link

rancher/norman#453 has been merged, but the go dependencies need to be updated. Could you please do that and re-push?

@sboschman
Copy link
Contributor Author

@ericpromislow as norman now requires go 1.22 and terraform-provider-rancher2 is on 1.19, perhaps it is better to bump the norman dep in a separate PR?

go: github.com/rancher/[email protected] requires go >= 1.22; switching to go1.22.2

From the drone ci logs:

Step 1/3 : FROM golang:1.19.4-alpine3.16

So, I assume bumping norman also requires some changes to the ci steps.

@ericpromislow
Copy link

@sboschman Thanks, I'll have a look

@ericpromislow
Copy link

Could you pull this out of draft mode so I can see what happens in CI? This repo doesn't have a make ci like most of the other rancher repo's so I can't easily verify it.

@ericpromislow
Copy link

Also I noticed the following problems with this repo with go 1.22 on both macos and ubuntu:

$ make build
…
go: modules disabled by GO111MODULE=off; see 'go help modules'
make: *** [lint] Error 1

AFAIK this variable is no longer needed. After I pull out the two GO111MODULE=X constructs from GNUmakefile and run this:

$ make build
# outputs go mod messages
$ go get -u
$ go mod tidy
$ make build

go vet complains about duplicate json tag type in both norman.Resource and v1.Secret embedded in type SecretV2 -- I don't know how this got passed earlier versions of go vet, and don't know if the t-p-r CI runs this.

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.

3 participants