-
-
Notifications
You must be signed in to change notification settings - Fork 101
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
WIndowsPb: gpg signature verification for ANT binary #3018
Conversation
I wouldnt expect there to be a problem with making directories in /home/$user once cygwin is installed |
Latest vpc hit the same error
Did not get this error when testing on test-azure-win2012r2-x64-1 |
I would guess that's because it's running under |
You can set an environment variable (There's probably a command line option to set it too, but I've not used that) |
Works in a cmd.exe environment too. I think with cygwin installed, any shell can access /home/ directories? |
Interesting ... the other thing to check would be whether there is another |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but my only comment would be that it may not be necessary to import the key each time (e.g. it could be skipped if it already exists on the machine)
Playbook checkers are failing, related? |
Looks like it. (My review was based on reading the code, and so the check failures need to be understood and addressed before this is merged) |
Create the home/.gnupg directory within windows/cygwin environment.
@Haroon-Khel needs a linter fix |
file: "{{playbook_dir}}/roles/GPG_signature_verification/tasks/main.yml" | ||
vars: | ||
file_path: c:\temp\ant.zip | ||
signature_link: "https://archive.apache.org/dist/ant/binaries/apache-ant-1.10.5-bin.zip.asc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ant is on 1.10.13 now - should we upgrade?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ant version
I would suggest that we put the security checks in place and then discuss that as a separate issue. I'd want there to be additional testing on a new ant version in order to ensure we don't have compatibility issues before just bumping it up (especially sine this PR is specific to Windows and we use the same fixed version across all platforms IIRC) - perhaps create a new issue proposing an upgrade? It's probably about time we did look at that.
|
Ant contrib is failing to download on the 2022 windows workflow
however that error is unrelated to this pr, the gpg check on the ant binary just before it passes without error A bit confused by this linter error
|
Linter issue is solved. The pr checks are running now because I had to rebase (branch was out of date) but this pr is ready to be merged |
The ant contrib error mentioned in #3018 (comment) is intermittent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ref #2908
It seems only the Ant download supplies a .asc or .sig file. For the other downloads, we supply a fixed checksum string which, since we do not plan on changing, provide adequate verification of a download.
Theres still the java downloads in the playbook which are capable of having gpg verification. Getting the signature file is a bit tricky, on unix we do something like this
which does not seem to be working well in a windows environment. So until I can figure this out, I thought id merge my work so far