Skip to content

Add URL query param support for verify_ssl#497

Open
florinutz wants to merge 1 commit into
crate:mainfrom
florinutz:respect-url-verify_ssl
Open

Add URL query param support for verify_ssl#497
florinutz wants to merge 1 commit into
crate:mainfrom
florinutz:respect-url-verify_ssl

Conversation

@florinutz

Copy link
Copy Markdown

Summary of the changes / Why this is an improvement

Adds support for ?verify_ssl=<bool> in the host URL passed to --hosts.

The cli flag --verify-ssl continues to take precedence over the URL param.

Checklist

@seut seut requested a review from amotl April 28, 2026 14:10

@amotl amotl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for the excellent contribution. Lgtm, but let me also add @mfussenegger for an additional review.

@amotl amotl requested a review from mfussenegger April 30, 2026 19:53
@mfussenegger

Copy link
Copy Markdown
Member

I hadn't seen #482
Can you elaborate on the motivation for this - why would we need both, --verify-ssl and a URL query arg?

@florinutz

Copy link
Copy Markdown
Author

we don't need both. @amotl wdyt?

@amotl

amotl commented May 4, 2026

Copy link
Copy Markdown
Contributor

Hi. I think it is more important for drivers, where you can't define any special flags. However, it is also convenient to be able to use the same style across environments, possibly making the handling easier for scripting purposes. I think GH-482 was written down after a conversation with @WalBeh coming from such spirits. Maybe he can drop a few words about his ambitions?

@mfussenegger

Copy link
Copy Markdown
Member

However, it is also convenient to be able to use the same style across environments, possibly making the handling easier for scripting purposes.

Not sure why - using a driver API and invoking the crash CLI is pretty different.
A few of the CI jobs for CrateDB actually do use a --verify-ssl False and use an URL that includes ?verify_ssl=False because they use both crash and cr8 and there are no issues with that.

Imho the PR isn't needed - more downsides to it due to the additional complexity, both in code but also from a UI perspective for users.

@amotl

amotl commented May 6, 2026

Copy link
Copy Markdown
Contributor

Thanks. @WalBeh: Could you elaborate why this was on your wishlist? Maybe it was a misunderstanding: In this case you can blame that on me.

@seut

seut commented May 27, 2026

Copy link
Copy Markdown
Member

@WalBeh ping

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.

Respect URL connectivity parameters

4 participants