allow to set https as the default scheme#8729
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8729 +/- ##
============================================
+ Coverage 69.66% 69.74% +0.08%
+ Complexity 4616 4614 -2
============================================
Files 1730 1730
Lines 90341 90325 -16
Branches 13442 13425 -17
============================================
+ Hits 62932 63001 +69
+ Misses 23045 22954 -91
- Partials 4364 4370 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Jackie-Jiang
added
the
release-notes
Jackie-Jiang
left a comment
Jackie-Jiang
left a comment
There was a problem hiding this comment.
Consider making it more explicit (as boolean)?
- controller.swagger.use.https
- pinot.broker.swagger.use.https
- pinot.server.swagger.use.https
- pinot.minion.swagger.use.https
| beanConfig.setVersion("1.0"); | ||
| beanConfig.setSchemes(new String[]{CommonConstants.HTTP_PROTOCOL, CommonConstants.HTTPS_PROTOCOL}); | ||
| if (CommonConstants.HTTPS_PROTOCOL.equalsIgnoreCase(_swaggerDefaultProtocol)) { | ||
| beanConfig.setSchemes(new String[]{CommonConstants.HTTPS_PROTOCOL, CommonConstants.HTTP_PROTOCOL}); |
There was a problem hiding this comment.
Should we still set HTTP?
There was a problem hiding this comment.
yeah, still needed at least for those quickstart examples, and perhaps for users not enabling TLS.
There was a problem hiding this comment.
oh, got your point. Taking the new configs suggested by you above, we ca save http here.
There was a problem hiding this comment.
hmm... on a second thought, I'd prefer to leave http there, in case someone wants to debug issues like when TLS doesn't work and he/she wants to switch to http for some quick checks.
| public static final String HELIX_CLUSTER_NAME = "controller.helix.cluster.name"; | ||
| public static final String CLUSTER_TENANT_ISOLATION_ENABLE = "cluster.tenant.isolation.enable"; | ||
| public static final String CONSOLE_WEBAPP_ROOT_PATH = "controller.query.console"; | ||
| public static final String CONSOLE_WEBAPP_DEFAULT_PROTOCOL = "controller.query.console.default.protocol"; |
There was a problem hiding this comment.
Why is controller key different from others? I don't think it applies to the query console
There was a problem hiding this comment.
I think this is some legacy naming convention, so I just kept it for consistency.
There was a problem hiding this comment.
but I'll take your controller.swagger.use.https
3 participants
This PR adds a switch to set 'https' as the default scheme for Swagger UI, so that in TLS enabled env, requests issued from Swagger UI will use https automatically.
Release notes
New configs are added for each Pinot component and followed the existing naming convention. By default,
httpis used as before.