search

hashicorp / terraform-provider-http

Utility provider for interacting with generic HTTP servers as part of a Terraform configuration.
https://registry.terraform.io/providers/hashicorp/http/latest
Mozilla Public License 2.0
206 stars 116 forks source link

Base64 encode response body #214

Closed ddelnano closed 1 year ago

ddelnano commented 1 year ago

This includes the changes in #158 with a temporary fix to address the CI failures. I was able to track this down to all versions of the v0.14.x releases and its related to the warning diag added.

While my testing proves that the failures on #158 are related to the diag, upon rebasing my branch I'm no longer able to reproduce the test failure. So it seems there is some interaction between the terraform version, terraform-plugin-sdk or terraform-plugin-framework version that causes the v0.14.x diag issues.

Testing

Your version of Terraform is out of date! The latest version is 1.3.6. You can update by downloading from https://www.terraform.io/downloads.html

Use original branch that was failing

ddelnano@ddelnano-desktop:~/code/terraform-provider-http$ git checkout bendbennett/issues-157 Switched to branch 'bendbennett/issues-157' Your branch is up to date with 'origin/bendbennett/issues-157'. ddelnano@ddelnano-desktop:~/code/terraform-provider-http$ make testacc 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.98s) === RUN TestDataSource_404 --- PASS: TestDataSource_404 (1.01s) === RUN TestDataSource_withAuthorizationRequestHeader_200 --- PASS: TestDataSource_withAuthorizationRequestHeader_200 (0.99s) === RUN TestDataSource_withAuthorizationRequestHeader_403 --- PASS: TestDataSource_withAuthorizationRequestHeader_403 (1.00s) === RUN TestDataSource_utf8_200 --- PASS: TestDataSource_utf8_200 (0.99s) === RUN TestDataSource_utf16_200 --- PASS: TestDataSource_utf16_200 (0.99s) === RUN TestDataSource_UpgradeFromVersion2_2_0 --- PASS: TestDataSource_UpgradeFromVersion2_2_0 (5.59s) === RUN TestDataSource_Provisioner === PAUSE TestDataSource_Provisioner === RUN TestDataSource_POST_201 --- PASS: TestDataSource_POST_201 (0.98s) === RUN TestDataSource_HEAD_204 --- PASS: TestDataSource_HEAD_204 (1.06s) === RUN TestDataSource_UnsupportedMethod --- PASS: TestDataSource_UnsupportedMethod (0.25s) === RUN TestDataSource_ResponseBodyText === PAUSE TestDataSource_ResponseBodyText === RUN TestDataSource_ResponseBodyBinary === PAUSE TestDataSource_ResponseBodyBinary === CONT TestDataSource_Provisioner === CONT TestDataSource_ResponseBodyBinary === CONT TestDataSource_ResponseBodyText === CONT TestDataSource_ResponseBodyBinary data_source_http_test.go:386: Step 1/1 error: Check failed: Check 1/2 error: Not found: data.http.http_test in [root] --- FAIL: TestDataSource_ResponseBodyBinary (0.90s) --- PASS: TestDataSource_ResponseBodyText (1.46s) --- PASS: TestDataSource_Provisioner (6.59s) FAIL coverage: 84.7% of statements FAIL github.com/terraform-providers/terraform-provider-http/internal/provider 20.428s ? github.com/terraform-providers/terraform-provider-http/terraform [no test files] FAIL make: *** [GNUmakefile:23: testacc] Error 1



@bendbennett would appreciate if we could get this merged since as I mentioned this would allow me to piggyback off an official provider's functionality rather than building this into the provider I maintain.
ddelnano commented 1 year ago

@bendbennett did you see my previous message? I'm very keen to get this merged so that I don't need to build something Xen Ochestra provider specific.

Please let me know if there is anything I can do to speed up the review process or someone else that is better to review this.

ddelnano commented 1 year ago

cc @bflad @austinvalle since I saw you merged a PR since I opened this one. Would appreciate if you could connect me with someone that can review this.

bookshelfdave commented 1 year ago

hello @ddelnano - base64 encoded response bodies are not on our roadmap at this point, although we may continue the work on the original PR some time in the next few months. As this provider is used by a very large number of practitioners, we have to prioritize stability over any enhancement. There's the possibility that this may actually be a breaking change, in which case, we have to plan well in advance to ensure minimal disruption. In the meantime, you can fork this repo to use these changes immediately.

ddelnano commented 1 year ago

Hey @bookshelfdave, thanks for getting back to me and I look forward to if/when your team revisits this functionality. In the meantime, I will publish a fork of this repo or build the functionality inside the xenorchestra provider

github-actions[bot] commented 4 months ago

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.