[release-4.8] OCPBUGS-5527: Fix label key truncation for subscription annotations (#2731) by openshift-cherrypick-robot · Pull Request #427 · openshift/operator-framework-olm

openshift-cherrypick-robot · 2023-01-10T02:59:56Z

This is an automated cherry-pick of #423

/assign awgreene

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> Co-authored-by: Per Goncalves da Silva <pegoncal@redhat.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: 1bdf969e6dff317e975de21078d60dd1406235e0

openshift-ci-robot · 2023-01-10T03:00:03Z

@openshift-cherrypick-robot: Jira Issue OCPBUGS-4948 has been cloned as Jira Issue OCPBUGS-5527. Retitling PR to link against new bug.
/retitle [release-4.8] OCPBUGS-5527: Fix label key truncation for subscription annotations (#2731)

Details

In response to this:

This is an automated cherry-pick of #423

/assign awgreene

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci · 2023-01-10T03:00:19Z

@openshift-cherrypick-robot: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Details

In response to this:

[release-4.8] OCPBUGS-5527: Fix label key truncation for subscription annotations (#2731)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2023-01-10T03:00:23Z

@openshift-cherrypick-robot: This pull request references Jira Issue OCPBUGS-5527, which is valid. The bug has been moved to the POST state.

6 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.8.z) matches configured target version for branch (4.8.z)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)
dependent bug Jira Issue OCPBUGS-4948 is in the state Closed (Done), which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE))
dependent Jira Issue OCPBUGS-4948 targets the "4.9.z" version, which is one of the valid target versions: 4.9.0, 4.9.z
bug has dependents

Requesting review from QA contact:
/cc @jianzhangbjz

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

This is an automated cherry-pick of #423

/assign awgreene

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

awgreene · 2023-01-10T03:02:51Z

@grokspawn we spoke about this before Holiday Shutdown, IIRC we had planned to backport this to 4.8 as some operator could hit this issue and SPAM the api server. Can you confirm that we had planned to backport this to 4.8?

awgreene · 2023-01-10T17:32:00Z

/retest

awgreene · 2023-01-16T20:53:12Z

/retest

awgreene · 2023-01-19T16:58:04Z

/retest

openshift-ci · 2023-01-19T20:01:10Z

@openshift-cherrypick-robot: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

awgreene · 2023-01-28T16:16:55Z

/label backport-risk-assessed

awgreene · 2023-01-28T16:17:18Z

/approve

awgreene · 2023-02-03T19:09:08Z

@jianzhangbjz could you review and add the cherry-pick-approved label?

dtfranz · 2023-02-03T19:12:27Z

/lgtm

tmshort

Depends on whether one has to worry about illegal characters on short names, or only when it's truncated.

tmshort · 2023-02-03T19:20:21Z

+
+		// Remove trailing illegal characters
+		idx := len(name) - 1
+		for ; idx >= 0; idx-- {
+			lastChar := name[idx]
+			if lastChar != '.' && lastChar != '_' && lastChar != '-' {
+				break
+			}
+		}
+
+		// Just being defensive. This is unlikely to happen since the operator name should
+		// be compatible kubernetes naming constraints
+		if idx < 0 {
+			return "", fmt.Errorf(componentLabelKeyError, "unsupported name field")
+		}
+
+		// Update Label
+		name = name[0 : idx+1]


Not sure why no one seems to:

if name = strings.TrimRight(name[0:63], "._-"); len(name) <= 0 { return .... }

Should the illegal characters be checked outside the len(name) check? Is it possible to set a name with an illegal trailing character? Could that happen?

Not sure why no one seems to:

if name = strings.TrimRight(name[0:63], "._-"); len(name) <= 0 {
return ....
}

I actually prefer your suggestion, but this is a backport of a fix and we attempt to avoid altering the code. We can always create a PR on master to do this though.

Is it possible to set a name with an illegal trailing character? Could that happen?

The kube api server will reject the create request and OLM will constantly try to create it even with the error, which is why we're backporting this.

tmshort · 2023-02-03T19:21:41Z

+			expectedLabel: "",
+			returnsError:  true,
+		},
+	}


This doesn't have the case for a small label key with illegal characters...

We'll need to fix this in master.

tmshort

/lgtm

openshift-ci · 2023-02-03T20:37:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: awgreene, openshift-cherrypick-robot, tmshort

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [awgreene]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jianzhangbjz · 2023-02-06T09:29:11Z

Test pass, details: https://issues.redhat.com/browse/OCPBUGS-5527
/lgtm
/label qe-approved
/label cherry-pick-approved

openshift-ci-robot · 2023-02-06T09:32:58Z

@openshift-cherrypick-robot: An error was encountered updating to the MODIFIED state for bug OCPBUGS-5527 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details.

Full error message.


No transition status with name `MODIFIED` could be found. Please select from the following list: []

Please contact an administrator to resolve this issue, then request a bug refresh with /jira refresh.

Details

In response to this:

This is an automated cherry-pick of #423

/assign awgreene

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

awgreene · 2023-02-06T14:55:05Z

/jira refresh

openshift-ci-robot · 2023-02-06T14:55:08Z

@awgreene: All pull requests linked via external trackers have merged:

openshift/operator-framework-olm#427

Jira Issue OCPBUGS-5527 has been moved to the MODIFIED state.

Details

In response to this:

/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-cherrypick-robot assigned awgreene Jan 10, 2023

openshift-cherrypick-robot mentioned this pull request Jan 10, 2023

[release-4.9] OCPBUGS-4948: Fix label key truncation for subscription annotations (#2731) #423

Merged

openshift-ci Bot changed the title ~~[release-4.8] OCPBUGS-4948: Fix label key truncation for subscription annotations (#2731)~~ [release-4.8] OCPBUGS-5527: Fix label key truncation for subscription annotations (#2731) Jan 10, 2023

openshift-ci Bot requested review from ankitathomas, dinhxuanvu and jianzhangbjz January 10, 2023 03:00

openshift-ci Bot added the backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. label Jan 28, 2023

openshift-ci Bot assigned bandrade, jianzhangbjz, kuiwang02 and Xia-Zhao-rh Jan 28, 2023

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 28, 2023

openshift-ci Bot assigned dtfranz Feb 3, 2023

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Feb 3, 2023

tmshort reviewed Feb 3, 2023

View reviewed changes

tmshort approved these changes Feb 3, 2023

View reviewed changes

openshift-ci Bot assigned tmshort Feb 3, 2023

openshift-ci Bot added qe-approved Signifies that QE has signed off on this PR cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. labels Feb 6, 2023

openshift-merge-robot merged commit 1250b91 into openshift:release-4.8 Feb 6, 2023

Conversation

openshift-cherrypick-robot commented Jan 10, 2023

Uh oh!

openshift-ci-robot commented Jan 10, 2023

Uh oh!

openshift-ci Bot commented Jan 10, 2023

Uh oh!

openshift-ci-robot commented Jan 10, 2023

Uh oh!

awgreene commented Jan 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

awgreene commented Jan 10, 2023

Uh oh!

awgreene commented Jan 16, 2023

Uh oh!

awgreene commented Jan 19, 2023

Uh oh!

openshift-ci Bot commented Jan 19, 2023

Uh oh!

awgreene commented Jan 28, 2023

Uh oh!

awgreene commented Jan 28, 2023

Uh oh!

awgreene commented Feb 3, 2023

Uh oh!

dtfranz commented Feb 3, 2023

Uh oh!

tmshort left a comment

Choose a reason for hiding this comment

Uh oh!

tmshort Feb 3, 2023

Choose a reason for hiding this comment

Uh oh!

awgreene Feb 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmshort Feb 3, 2023

Choose a reason for hiding this comment

Uh oh!

awgreene Feb 3, 2023

Choose a reason for hiding this comment

Uh oh!

tmshort left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Feb 3, 2023

Uh oh!

jianzhangbjz commented Feb 6, 2023

Uh oh!

openshift-ci-robot commented Feb 6, 2023

Uh oh!

awgreene commented Feb 6, 2023

Uh oh!

openshift-ci-robot commented Feb 6, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

awgreene commented Jan 10, 2023 •

edited

Loading

awgreene Feb 3, 2023 •

edited

Loading