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

fix: warn but don't error out on unsupported http response on older ansible versions #507

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

gardar
Copy link
Member

@gardar gardar commented Jan 7, 2025

GitLab has recently started using 308 redirects for release assets, which causes compatibility issues with older versions of Ansible and Python. Our options for resolving this are limited, so the best solution for now is to allow the checksum download task to proceed with a warning. Unfortunately, this has the side effect of disabling checksum verification when the warning is triggered, but AFAIK it remains the only workable approach at this time.

See more here:
#475 (comment)
https://github.com/prometheus-community/ansible/actions/runs/11955696431/job/33328750307
cc @mgariepy

@SuperQ
Copy link
Contributor

SuperQ commented Jan 7, 2025

What versions of python/ansible are unsupported?

Copy link
Contributor

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Read through the comments, I guess this is fine for now.

@gardar
Copy link
Member Author

gardar commented Jan 7, 2025

Looks like it's ansible 2.9-2.13
https://github.com/prometheus-community/ansible/actions/runs/11955696431/job/33328750307

btw. I was able to get a 302 response from gitlab by forcing http1.0 but unfortunately there doesn't seem to set the http version in the ansible module, I tried various headers to try and trick it to give a http1.0 response but no avail.

@github-actions github-actions bot added bugfix and removed bugfix labels Jan 7, 2025
@github-actions github-actions bot added bugfix and removed bugfix labels Jan 7, 2025
@gardar
Copy link
Member Author

gardar commented Jan 7, 2025

Was able to fix the binary download itself by adding a fallback download task with the uri ansible module, we could fully replace the get_url and 'url' lookup tasks with uri module to avoid the 308 issue but then we'd lose any checksum verifications.

@mgariepy
Copy link

mgariepy commented Jan 7, 2025

wow, it seems to works i wonder if we could verify checksum in an extra task during the rescue, just to keep integrity.

@gardar
Copy link
Member Author

gardar commented Jan 10, 2025

wow, it seems to works i wonder if we could verify checksum in an extra task during the rescue, just to keep integrity.

Good point! I added a separate task for checksum verification, that enabled me to replace get_url in the download task with uri so that we don't actually need to add the fallback download for the old ansible versions.

@gardar gardar force-pushed the workaround-gitlab-308-redirect branch from 1a395bc to 6209b53 Compare January 10, 2025 20:57
@github-actions github-actions bot added bugfix and removed bugfix labels Jan 10, 2025
@mgariepy
Copy link

wow, it seems to works i wonder if we could verify checksum in an extra task during the rescue, just to keep integrity.

Good point! I added a separate task for checksum verification, that enabled me to replace get_url in the download task with uri so that we don't actually need to add the fallback download for the old ansible versions.

it's also needed when using older python with new-ish ansible.

Thanks a lot for your patches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment