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

Base64 encode response body #214

Closed
wants to merge 6 commits into from

Conversation

ddelnano
Copy link

@ddelnano ddelnano commented Dec 21, 2022

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

  • make testacc passes when terraform v0.14.x is installed in my PATH (fails without the commented out diag)
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 (1.00s)
=== RUN   TestDataSource_404
--- PASS: TestDataSource_404 (0.98s)
=== RUN   TestDataSource_withAuthorizationRequestHeader_200
--- PASS: TestDataSource_withAuthorizationRequestHeader_200 (0.99s)
=== RUN   TestDataSource_withAuthorizationRequestHeader_403
--- PASS: TestDataSource_withAuthorizationRequestHeader_403 (0.98s)
=== RUN   TestDataSource_utf8_200
--- PASS: TestDataSource_utf8_200 (0.98s)
=== RUN   TestDataSource_utf16_200
--- PASS: TestDataSource_utf16_200 (0.99s)
=== RUN   TestDataSource_UpgradeFromVersion2_2_0
--- PASS: TestDataSource_UpgradeFromVersion2_2_0 (5.58s)
=== RUN   TestDataSource_Provisioner
=== PAUSE TestDataSource_Provisioner
=== RUN   TestDataSource_POST_201
--- PASS: TestDataSource_POST_201 (1.00s)
=== RUN   TestDataSource_HEAD_204
--- PASS: TestDataSource_HEAD_204 (1.01s)
=== 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
--- PASS: TestDataSource_ResponseBodyBinary (1.49s)
--- PASS: TestDataSource_ResponseBodyText (1.51s)
--- PASS: TestDataSource_Provisioner (6.60s)
PASS
coverage: 84.5% of statements
ok      github.com/terraform-providers/terraform-provider-http/internal/provider        20.375s coverage: 84.5% of statements
?       github.com/terraform-providers/terraform-provider-http/terraform        [no test files]
  • make testacc fails when it's uncommented with terraform v0.14.x
ddelnano@ddelnano-desktop:~/code/terraform-provider-http$ terraform --version
Terraform v0.14.9

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
Copy link
Author

@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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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

Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants