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.

Sign up for GitHub

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

Jump to bottom

Add TLS CLI commands #41418

Merged
merged 1 commit into from
Jul 6, 2024
Merged

Add TLS CLI commands #41418

merged 1 commit into from
Jul 6, 2024

Conversation

cescoffier
Copy link
Member

This PR introduces two new CLI commands to enhance TLS management:

  • Generate and install a Quarkus Development CA.
  • Create certificates, either signed with the generated CA or unsigned.

Fix #41010

@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/documentation labels Jun 25, 2024
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 27, 2024 •
edited
Loading

🙈 The PR is closed and the preview is expired.

@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@cescoffier cescoffier mentioned this pull request Jul 1, 2024
Merged
@geoand geoand requested a review from iocanel July 1, 2024 10:40
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
gsmet
gsmet requested changes Jul 2, 2024
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Sorry it took so long.

Most of the comments are related to the doc but... there are some other important comments.

docs/src/main/asciidoc/tls-registry-reference.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/tls-registry-reference.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/tls-registry-reference.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/tls-registry-reference.adoc Show resolved Hide resolved
docs/src/main/asciidoc/tls-registry-reference.adoc
alias in the keystore
-p, --password=<password>
The password of the keystore. Default is 'password'
-r, --renew Whether existing certificates will need to be replaced
Copy link
Member

Choose a reason for hiding this comment

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

Would it have value to be able to generate a certificate from a given CA?

Copy link
Member Author

Choose a reason for hiding this comment

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

you mean a CA you already own? I would need the private key. I would rather not do that for security reasons.

docs/src/main/asciidoc/tls-registry-reference.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/tls-registry-reference.adoc Outdated Show resolved Hide resolved
extensions/tls-registry/cli/src/main/java/io/quarkus/tls/cli/GenerateCertificateCommand.java Outdated Show resolved Hide resolved
docs/src/main/asciidoc/tls-registry-reference.adoc Outdated Show resolved Hide resolved
build-parent/pom.xml Outdated
Comment on lines 375 to 379
<dependency>
<groupId>me.escoffier.certs</groupId>
<artifactId>certificate-generator</artifactId>
<version>0.7.1</version>
</dependency>
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 we should move these projects to the Quarkus or the SmallRye org. And same for the groupId.

For tests, it was mostly OK but I wouldn't be comfortable with having some CLI/runtime stuff hosted outside of our community namespaces. And this especially since they are security components.

Both for security and for the future.

I won't require this to get the PR in but we need to have a plan and a deadline for it.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's still the goal. I would go to SmallRye as the project has a lot of tests, and I don't want to use Quarkus CI time for this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will work on something tomorrow.

Copy link
Member Author

Choose a reason for hiding this comment

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

See #41665

Do not resolve this conversation - once the mentioned PR is merged, there is some work to do to transition to this new dependency.

@quarkus-bot quarkus-bot bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/platform Issues related to definition and interaction with Quarkus Platform labels Jul 2, 2024
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
iocanel
iocanel reviewed Jul 3, 2024
View reviewed changes
extensions/tls-registry/runtime/src/main/resources/META-INF/quarkus-extension.yaml Outdated
@@ -8,3 +8,5 @@ metadata:
unlisted: true
config:
- "quarkus.tls."
cli-plugins:
- "${project.groupId}:quarkus-tls-registry-cli:${project.version}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- "${project.groupId}:quarkus-tls-registry-cli:${project.version}"
- tls: "${project.groupId}:quarkus-tls-registry-cli:${project.version}"

Copy link
Contributor

Choose a reason for hiding this comment

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

As of #40580 we can prefix the maven coordinates with an alias.
In this case tls: .

Copy link
Member Author

Choose a reason for hiding this comment

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

Awesome!

Copy link
Member Author

Choose a reason for hiding this comment

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

Did the changes.

  cli-plugins:
  - "tls: ${project.groupId}:quarkus-tls-registry-cli:${project.version}"

iocanel
iocanel reviewed Jul 3, 2024
View reviewed changes
docs/src/main/asciidoc/tls-registry-reference.adoc
> quarkus tls-registry
Install and Manage TLS development certificates
Usage: tls-registry [COMMAND]
Commands:
Copy link
Contributor

Choose a reason for hiding this comment

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

We could optionally pull the generate- prefix as an intermediate sub-command:

quarkus tls generate qukarus-ca
quarkus tls generate certificate
```

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. What's the recommendation?

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 would keep it as it is for now. They are fundamentally different operations.

iocanel
iocanel approved these changes Jul 3, 2024
View reviewed changes
Copy link
Contributor

@iocanel iocanel left a comment

Choose a reason for hiding this comment

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

LGTM

@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@cescoffier cescoffier mentioned this pull request Jul 3, 2024
Merged
@gsmet gsmet added triage/waiting-for-ci Ready to merge when CI successfully finishes release/noteworthy-feature and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels Jul 4, 2024
@gsmet
Copy link
Member

gsmet commented Jul 4, 2024

@cescoffier I think you will need to update this one with the new artifacts.

@cescoffier
Copy link
Member Author

@gsmet Yes! On it!

@cescoffier
Add TLS Command Implementation
da8d7a9 
This commit introduces two new CLI commands to enhance TLS management:

- Generate and install a Quarkus Development CA.
- Create certificates, either signed with the generated CA or unsigned.

As the generated certificates are generated in ``$project-directory/.certs`, this directory is added to the generated .gitignore.

The CLI uses the new alias supported added in quarkusio#40580, and thus the commands are behind: `quarkus tls`
@cescoffier
Copy link
Member Author

@gsmet Done.

@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Jul 5, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit da8d7a9.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

⚠️ There are other workflow runs running, you probably need to wait for their status before merging.

@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Jul 5, 2024 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit da8d7a9.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@gsmet gsmet merged commit 98b2c06 into quarkusio:main Jul 6, 2024
55 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.13 - main milestone Jul 6, 2024
@cescoffier cescoffier deleted the tls-ca-commands branch July 7, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsmet gsmet gsmet approved these changes

@iocanel iocanel iocanel approved these changes

Assignees
No one assigned
Labels
area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/platform Issues related to definition and interaction with Quarkus Platform release/noteworthy-feature triage/flaky-test
Projects
Quarkus Documentation
Status: Done
WG - Enhanced TLS support
Status: Done
Milestone
3.13.0.CR1
Development

Successfully merging this pull request may close these issues.

CLI plugin to generate certificates
3 participants
@cescoffier @gsmet @iocanel