Make sure truncated labels are valid #1055
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Ruben Vargas <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1055 +/- ##
==========================================
+ Coverage 64.58% 64.70% +0.11%
==========================================
Files 84 85 +1
Lines 6715 6904 +189
==========================================
+ Hits 4337 4467 +130
- Misses 2236 2279 +43
- Partials 142 158 +16
Continue to review full report at Codecov.
|
objectiser
reviewed
May 11, 2020
objectiser
approved these changes
May 13, 2020
pkg/util/truncate.go
Outdated
| func init() { | ||
| regexpEndReplace, _ = regexp.Compile("[^A-Za-z0-9]+$") | ||
| regexpBeginReplace, _ = regexp.Compile("^[^A-Za-z0-9]+") | ||
|
|
There was a problem hiding this comment.
nit: Can you remove this line.
Signed-off-by: Ruben Vargas <[email protected]>
objectiser
approved these changes
May 14, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Ruben Vargas [email protected]
While I was working on fix token propagation on OLM I saw a couple of errors on operator logs:
time="2020-05-07T16:28:25Z" level=error msg="failed to apply the changes" error="ClusterRoleBinding.rbac.authorization.k8s.io \"osdk-e2e-3bed6aed-720f-4eca-9cbc-7aa27c919f34-token-prop-oauth-proxy-auth-delegator\" is invalid: metadata.labels: Invalid value: \"osdk-e2e-3bed6aed-720f-4eca-9cbc-7aa27c919f34-token-prop-oauth-\": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')" execution="2020-05-07 16:28:25.056741381 +0000 UTC" instance=token-prop namespace=osdk-e2e-3bed6aed-720f-4eca-9cbc-7aa27c919f34I realized that this was due the truncation of labels to a maximum of 63 characters, That truncation process could produces labels that ends with "-" (at least in this case). A quick fix was to change the name of the Jaeger instance in token propagation tests to
tokenpropso all the truncated label ends with an alphanumeric. I think that is not a definitive way to fix the issue, so I created this PR.