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 private cert issues discovered in manual testing #654

Merged
merged 8 commits into from
Feb 10, 2023

Conversation

michaelsauter
Copy link
Member

@michaelsauter michaelsauter commented Feb 7, 2023

This is a follow-up from #648. To make it easier to test #648 manually, we merged that before doing manual testing.

The issues addressed in this PR were not picked up in automated tests because:

  • when -private-cert is given, the tests clone from a server using the test private cert, but in manual testing the server was using a public cert (and only Nexus/SQ were using private certs). While debugging this issue I replaced the usage of Tekton's git-init with plain git, making it easier to debug what is going on and removing a dependency in the container image that is not really needed.
  • The md5 logic tests used the md5 binary, and the md5sum binary in the UBI images behaves differently than expected, I got that mixed up when I tried md5sum locally in an UBI image earlier.
  • The Gradle configure script was using some old paths, I assume I updated them after running the tests locally, and the GH tests do not actually execute the private cert path right now. Long-term we should enable this somehow.

Tasks:

  • Updated design documents in docs/design directory or not applicable
  • Updated user-facing documentation in docs directory or not applicable
  • Ran tests (e.g. make test) or not applicable
  • Updated changelog or not applicable

Simplifies the container image and makes it easier to follow what is
going on.
Allows to pull from servers using a public cert as well.
@michaelsauter michaelsauter self-assigned this Feb 7, 2023
@michaelsauter michaelsauter changed the title Fix TLS issues discovered in manual testing Fix private cert issues discovered in manual testing Feb 7, 2023
CONTENT+="systemProp.javax.net.ssl.trustStore=.ods-cache/keystore/cacerts\n"
CONTENT+="systemProp.javax.net.ssl.trustStorePassword=password\n"
truststore_location="$(pwd)/.ods-cache/truststore/cacerts"
truststore_pass="changeit"
Copy link
Member

Choose a reason for hiding this comment

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

I think I am lost, where does this truststore_pass ultimately come from?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's the default in the UBI image .... it is never actually "changed".

Copy link
Member

@henrjk henrjk left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@michaelsauter michaelsauter merged commit a6b6ccb into master Feb 10, 2023
@michaelsauter michaelsauter deleted the fix/tls-issues branch February 10, 2023 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants