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

getproviders: account for occasionally missing Host header in errors #31542

Merged
merged 1 commit into from Jul 29, 2022

Conversation

radeksimko
Copy link
Member

@radeksimko radeksimko commented Jul 29, 2022

This is a follow-up PR to #30810 and #29298

They each aimed to add more context to the following error messages:

Error while installing citrix/citrixadc v1.13.0: could not query provider
registry for registry.terraform.io/citrix/citrixadc: failed to retrieve
authentication checksums for provider: 403 Forbidden

Error: Failed to install provider
Error while installing 1password/onepassword v1.1.2: could not query provider
registry for registry.terraform.io/1password/onepassword: failed to retrieve
authentication checksums for provider: the request failed after 2 attempts,
please try again later: 500

Based on some recent CI runs, it became clear that Request.Host may sometimes be empty, which results in the following error message:

Error: Failed to install provider

Error while installing assistanz/stackbill v0.1.0: could not query provider
registry for registry.terraform.io/assistanz/stackbill: failed to retrieve
cryptographic signature for provider: 403 Forbidden returned from 

See https://github.com/hashicorp/terraform-ls/runs/7574713438?check_suite_focus=true#step:6:46

I don't actually know under what exact circumstances would the Host be missing. It is clear however from reading the implementation of net/http.Request.Write() that this may happen.

I therefore propose to mimic what the stdlib does here, to ensure we always expose the host in the error message.

I expect #31524 can be updated to take advantage of this too, once this PR is merged.

@radeksimko radeksimko changed the title getproviders: account for occasionally missing Host header in errors getproviders: account for occasionally missing Host header in errors Jul 29, 2022
Copy link
Member

@kmoe kmoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@radeksimko radeksimko merged commit b82eee6 into main Jul 29, 2022
@radeksimko radeksimko deleted the getproviders-account-for-missing-host branch July 29, 2022 09:23
@github-actions
Copy link

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

@github-actions
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 Aug 29, 2022
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.

None yet

2 participants