Add util for finding credential helper to use#15707
Add util for finding credential helper to use#15707Yannic wants to merge 3 commits intobazelbuild:masterfrom
Conversation
Yannic
force-pushed
the
yannic-creds-helper-provider
branch
from
ac79963 to
2f8c78e
Compare
Yannic
force-pushed
the
yannic-creds-helper-provider
branch
from
2f8c78e to
483bbd7
Compare
|
@tjgq PTAL |
sgowroji
added
team-ExternalDeps
| () -> { | ||
| int dot = host.indexOf('.'); | ||
| if (dot < 0) { | ||
| // We reached the last segment, end. | ||
| return Optional.empty(); | ||
| } | ||
| return findWildcardCredentialHelper(host.substring(dot + 1)); |
There was a problem hiding this comment.
I think it would be clearer to encapsulate this logic in a boolean hostMatchesPattern(String pattern, String host) helper.
There was a problem hiding this comment.
Slightly refactored the "get parent domain" logic into helper function.
| if (pattern.startsWith("*.")) { | ||
| wildcards.put(pattern.substring(2), helper); | ||
| } else { | ||
| hosts.put(pattern, helper); |
There was a problem hiding this comment.
I think we should do more exhaustive validation to reject stuff like foo.*.bar, http://foo.bar, http://foo.bar/path/to/resource, or http://foo.bar:8080.
To be concrete, I think we want the pattern to match the regex (\*|[-a-zA-Z0-9]+)(\.[-a-zA-Z0-9]+)+ (technically domain names are stricter, but this should cover most misuses).
There was a problem hiding this comment.
Done (+ also handling non-ascii DNS names correctly)
| if (pattern.startsWith("*.")) { | ||
| wildcards.put(pattern.substring(2), helper); | ||
| } else { | ||
| hosts.put(pattern, helper); |
There was a problem hiding this comment.
Done (+ also handling non-ascii DNS names correctly)
| () -> { | ||
| int dot = host.indexOf('.'); | ||
| if (dot < 0) { | ||
| // We reached the last segment, end. | ||
| return Optional.empty(); | ||
| } | ||
| return findWildcardCredentialHelper(host.substring(dot + 1)); |
There was a problem hiding this comment.
Slightly refactored the "get parent domain" logic into helper function.
|
@tjgq ping? |
|
FYI, I fixed a few of the test assertions to use |
|
@bazel-io flag |
bazel-io
added
the
potential release blocker
|
@bazel-io fork 5.3.0 |
bazel-io
removed
the
potential release blocker
Progress on https://github.com/bazelbuild/proposals/blob/main/designs/2022-06-07-bazel-credential-helpers.md Closes #15707. PiperOrigin-RevId: 458456496 Change-Id: I751a594144c3563096ee9794c41329b49755824e Co-authored-by: Yannic Bonenberger <[email protected]>
ShreeM01
removed
the
awaiting-review
6 participants
Progress on https://github.com/bazelbuild/proposals/blob/main/designs/2022-06-07-bazel-credential-helpers.md