terraform-provider-http
terraform-provider-http copied to clipboard
add CA certificate verification and insecure option
Hi,
this PR is 1:1 copy of #29. The original author looks inactive and is not able to sign the CLA. To unblock the process, I'm create new PR here.
This PR will let user validate TLS certificate with a provided root CA or choose an insecure option with no validation at all.
May solves https://github.com/hashicorp/terraform-provider-aws/pull/11426 and have a lot of common usage.
Link for tests https://github.com/jkroepke/terraform-provider-http/pull/1
Closes: #29
- [x] Create an issue that
insecureshould conflicts withca_cert_pem.
I'm active. But the way Hashicorp is dealing with this repository make me feel that they don't need further contributions on this. #29 is 2 years old with lot of effort of keeping the PR up to date. I think I've already signed this CLA twice since I opened this PR. So I ended by forking this into https://github.com/terraform-aws-modules/terraform-provider-http.
BTW, feel free to maintain this one and I hope that someone at Hashicorp will get this merged.
@detro I would like to pull you for a review here, since I saw some recent activity on your side. In case you are too busy, please ping others.
Thanks.
@barryib I saw your fork and currently l‘m using it.
Since you are remove the forked provider from the eks module, there is some risk that the fork will be deprecated.
Rebased.
Rebased on #142
@bendbennett Can we have an review/merge soon?
Hi @bendbennett
thanks for the review. I apply all the suggestion except on the ExceptError. since giving the URL would not work without any escape here.
Changed applied. make test is fine.
jkr@joe-nb terraform-provider-http % make test
go test -v -cover -timeout=120s -parallel=4 ./...
? github.com/terraform-providers/terraform-provider-http [no test files]
=== RUN TestDataSource_200
--- PASS: TestDataSource_200 (0.99s)
=== RUN TestDataSource_404
--- PASS: TestDataSource_404 (0.90s)
=== RUN TestDataSource_withAuthorizationRequestHeader_200
--- PASS: TestDataSource_withAuthorizationRequestHeader_200 (0.77s)
=== RUN TestDataSource_withAuthorizationRequestHeader_403
--- PASS: TestDataSource_withAuthorizationRequestHeader_403 (0.80s)
=== RUN TestDataSource_utf8_200
--- PASS: TestDataSource_utf8_200 (0.74s)
=== RUN TestDataSource_utf16_200
--- PASS: TestDataSource_utf16_200 (0.73s)
=== RUN TestDataSource_x509cert
--- PASS: TestDataSource_x509cert (0.73s)
=== RUN TestDataSource_UpgradeFromVersion2_2_0
data_source_http_test.go:177: Acceptance tests skipped unless env 'TF_ACC' set
--- SKIP: TestDataSource_UpgradeFromVersion2_2_0 (0.00s)
=== RUN TestDataSource_WithCACertificate
=== PAUSE TestDataSource_WithCACertificate
=== RUN TestDataSource_InsecureTrue
=== PAUSE TestDataSource_InsecureTrue
=== RUN TestDataSource_InsecureFalse
=== PAUSE TestDataSource_InsecureFalse
=== RUN TestDataSource_InsecureUnconfigured
=== PAUSE TestDataSource_InsecureUnconfigured
=== CONT TestDataSource_WithCACertificate
=== CONT TestDataSource_InsecureFalse
=== CONT TestDataSource_InsecureTrue
=== CONT TestDataSource_InsecureUnconfigured
--- PASS: TestDataSource_InsecureUnconfigured (0.22s)
--- PASS: TestDataSource_InsecureFalse (0.22s)
--- PASS: TestDataSource_InsecureTrue (0.86s)
--- PASS: TestDataSource_WithCACertificate (0.86s)
PASS
coverage: 85.1% of statements
ok github.com/terraform-providers/terraform-provider-http/internal/provider 6.845s coverage: 85.1% of statements
I will add an additional test case for invalid certificates.
CHANGELOG.md entry added.
@bendbennett I hope, everything is fine now.
@jkroepke looks like the linter is complaining. Are you happy to fix the linting issues?
@bendbennett I hope I get all lint issues.
About the workflow approval condition. Consider to select Require approval for first-time contributors who are new to GitHub instead Require approval for first-time contributors.
Now I have to wait for an approval again to see the results from golintci.
See: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#controlling-changes-from-forks-to-workflows-in-public-repositories
@jkroepke apologies for any inconvenience caused but our current security policies require that the workflow settings are configured to Require approval for first-time contributors.
Alright, then lets see what the next runs says.
Currently, I have no idea whats going wrong here. I'm unable to reproduce the error locally.
TF_ACC=1 go test -v -cover -timeout 120m ./...
? github.com/terraform-providers/terraform-provider-http [no test files]
=== RUN TestDataSource_200
--- PASS: TestDataSource_200 (0.76s)
=== RUN TestDataSource_404
--- PASS: TestDataSource_404 (0.74s)
=== RUN TestDataSource_withAuthorizationRequestHeader_200
--- PASS: TestDataSource_withAuthorizationRequestHeader_200 (0.75s)
=== RUN TestDataSource_withAuthorizationRequestHeader_403
--- PASS: TestDataSource_withAuthorizationRequestHeader_403 (0.74s)
=== RUN TestDataSource_utf8_200
--- PASS: TestDataSource_utf8_200 (0.75s)
=== RUN TestDataSource_utf16_200
--- PASS: TestDataSource_utf16_200 (0.78s)
=== RUN TestDataSource_x509cert
--- PASS: TestDataSource_x509cert (0.74s)
=== RUN TestDataSource_UpgradeFromVersion2_2_0
--- PASS: TestDataSource_UpgradeFromVersion2_2_0 (3.75s)
=== RUN TestDataSource_WithCACertificate
=== PAUSE TestDataSource_WithCACertificate
=== RUN TestDataSource_WithCACertificateFalse
=== PAUSE TestDataSource_WithCACertificateFalse
=== RUN TestDataSource_InsecureTrue
=== PAUSE TestDataSource_InsecureTrue
=== RUN TestDataSource_InsecureFalse
=== PAUSE TestDataSource_InsecureFalse
=== RUN TestDataSource_InsecureUnconfigured
=== PAUSE TestDataSource_InsecureUnconfigured
=== CONT TestDataSource_WithCACertificate
=== CONT TestDataSource_InsecureFalse
=== CONT TestDataSource_InsecureTrue
=== CONT TestDataSource_WithCACertificateFalse
=== CONT TestDataSource_InsecureUnconfigured
--- PASS: TestDataSource_WithCACertificateFalse (0.23s)
--- PASS: TestDataSource_InsecureFalse (0.25s)
--- PASS: TestDataSource_InsecureUnconfigured (0.25s)
--- PASS: TestDataSource_InsecureTrue (0.85s)
--- PASS: TestDataSource_WithCACertificate (0.90s)
PASS
coverage: 86.7% of statements
ok github.com/terraform-providers/terraform-provider-http/internal/provider (cached) coverage: 86.7% of statements
Edit: It seem like the error messages depends on the used golang version.
go1.17:
Error making request: Get "https://127.0.0.1:49383/": x509: certificate signed
by unknown authority
go1.18:
Error making request: Get "https://127.0.0.1:49383/": x509: “Acme Co”
certificate is not trusted
Based on the error
Error tls: crypto/x509: system root pool is not available on Windows
from windows systems, I not longer reference the system root pool
@bendbennett Tests are green.
@detro @bendbennett Is this PR fine? Can I assist here to move forward?
Edit: It seem like the error messages depends on the used golang version.
go1.17:
Error making request: Get "https://127.0.0.1:49383/": x509: certificate signed by unknown authoritygo1.18:
Error making request: Get "https://127.0.0.1:49383/": x509: “Acme Co” certificate is not trusted
@jkroepke sorry for the delay in getting back to you.
Given that the README states that Go 1.17 is a requirement, it's probably best that the test is modified along the following lines so that the test will pass when using Go 1.17 and will not break when we upgrade to 1.18:
ExpectError: regexp.MustCompile(fmt.Sprintf(`Error making request: Get "%s": x509: `, svr.URL)),
I hope, everything is fine now. :-)
Branch rebased on main.
Rebased.
@bendbennett can we merge this PR soon?
@bendbennett Can I help here to move forward here?
@jkroepke apologies for the delay in responding. We are just finalising some other changes to the http provider. We will get back to you as soon as we can.
I would much appreciate it, if this can. get merged soon.
hello @jkroepke - thank you for your contribution and apologies for the delay, we'll be able to put more energy into this over the next few weeks.
Rebased. all comments addressed. Tests for the Validators added.
I hope, we are complete here now.
It feels like the schemavalidator.ConflictsWith results into problems on 0.14?
No idea, whats wrong here...
edit: It should work now. On my local machine. 0.14 is green now.
woohoo, it made it! thanks @jkroepke and maintainers 🎉
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.