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

Add support for Quarkus CLI TLS command #1268

Merged
merged 1 commit into from
Aug 30, 2024

Conversation

michalvavrik
Copy link
Member

Summary

We need to test https://quarkus.io/guides/tls-registry-reference#quarkus-cli-commands-and-development-ca-certificate-authority so adding few convenient methods to facilitate testing.

Please check the relevant options

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Dependency update
  • Refactoring
  • Release (follows conventions described in the RELEASE.md)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change requires a documentation update
  • This change requires execution against OCP (use run tests phrase in comment)

Checklist:

  • Example scenarios has been updated / added
  • Methods and classes used in PR scenarios are meaningful
  • Commits are well encapsulated and follow the best practices

Copy link
Member

@rsvoboda rsvoboda left a comment

Choose a reason for hiding this comment

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

Approving, just 2 minor things that are not mandatory for merging.
I will wait till CI is done.

@michalvavrik
Copy link
Member Author

I will wait till CI is done.

Just FYI I run all related tests locally so I hope it will be fine. But sure, no merging till it's green.

@rsvoboda
Copy link
Member

examples/quarkus-cli fails with 999-SNAPSHOT

@rsvoboda
Copy link
Member

Error: 6,695 INFO [ERROR] TlsCommandTest.testKeystoreInDefaultTlsRegistry:31 NullPointer Cannot invoke "java.security.KeyStore.aliases()" because "ks" is null

 13:47:08,831 INFO  /home/runner/work/quarkus-test-framework/quarkus-test-framework/quarkus-dev-cli tls generate-certificate -c Dumbledore --name=dev-certificate --*** -d /home/runner/work/quarkus-test-framework/quarkus-test-framework/examples/quarkus-cli/target/quarkus-tls-command-tests/654ba4d1-604c-4402-99a9-05bcd15a67ff/tls-command-test
13:47:12,832 INFO  Command tls is not available, looking for available plugins ...
Error: 2,832 INFO  [ERROR] ❗  Command tls is missing and can't be installed.
Error: 2,832 INFO  [ERROR] ❗  Unmatched arguments from index 0: 'tls', 'generate-certificate', '-c', 'Dumbledore', '--name=dev-certificate', '--***', '-d', '/home/runner/work/quarkus-test-framework/quarkus-test-framework/examples/quarkus-cli/target/quarkus-tls-command-tests/654ba4d1-604c-4402-99a9-05bcd15a67ff/tls-command-test'

@rsvoboda
Copy link
Member

tls is plugin of cli, maybe that's a problem?

@michalvavrik
Copy link
Member Author

The cause is here: Command tls is missing and can't be installed. which I don't know why works locally and not in CI. I simply have to look what is different as I build CLI with same commands as CI does.

@michalvavrik
Copy link
Member Author

tls is plugin of cli, maybe that's a problem?

it needs Quarkus TLS Registry extension, which is present though. I'll find the difference and write it here.

@michalvavrik
Copy link
Member Author

I have reproduced it in brand new fork few times. Funny business, it seems that same command fails depending on parent directories of a Quarkus application created with CLI command. Then I deleted that fork and started a new fork and can't reproduce it again. There is probably some algorithm to it. I'll try to find reproducer.

@michalvavrik
Copy link
Member Author

What I have changed is to perform test in temporary directory instead. I hope it's alright for now.

@michalvavrik
Copy link
Member Author

Okay, I am thinking it is about some lock when the command is first executed during test because I cannot reproduce it outside of it.

@michalvavrik michalvavrik added the triage/backport-1.5? Quarkus 3.15 stream label Aug 30, 2024
@michalvavrik
Copy link
Member Author

CI is green, merging.

@michalvavrik michalvavrik merged commit 590fe21 into quarkus-qe:main Aug 30, 2024
8 checks passed
@michalvavrik michalvavrik deleted the feature/tls-cli-command branch August 30, 2024 17:11
@michalvavrik michalvavrik mentioned this pull request Sep 3, 2024
11 tasks
@michalvavrik michalvavrik removed the triage/backport-1.5? Quarkus 3.15 stream label Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants