Bug 2050332: explicit Pattern to validate Duration fields

[2uasimojo]

Due to a difference in how apiserver validates Format=duration vs how
apimachinery unmarshals metav1.Duration, using that format can allow
users to blow up some of our controllers if they provide certain values
for Duration fields (specifically: containing d or w units).
Upstream issues have been opened against both sides of this discrepancy:

• metav1.Duration doesn't parse d, w; conflicts with kubebuilder format=duration validation kubernetes/apimachinery#131
• format=duration kubebuilder validation accepts d, w; metav1.Duration does not kubernetes/apiextensions-apiserver#56

In the meantime, work around the problem by using an explicit regex
instead of the built in format validator.

Closes 2050332

[openshift-ci]

@2uasimojo: This pull request references Bugzilla bug 2050332, which is invalid:

• expected the bug to target the "4.11.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 2050332: explicit Pattern to validate Duration fields

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.

[2uasimojo]

/bugzilla refresh

[openshift-ci]

@2uasimojo: This pull request references Bugzilla bug 2050332, which is invalid:

• expected the bug to target the "4.11.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla 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.

[2uasimojo]

I don't know how to make the bz "target 4.11.0". The only thing I see with "target" is "Target Milestone: ---", which I can't seem to edit. @akhil-rane can you help?

[akhil-rane]

/bugzilla refresh

[openshift-ci]

@akhil-rane: This pull request references Bugzilla bug 2050332, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target release (4.11.0) matches configured target release for branch (4.11.0)
• bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @lwan-wanglin

In response to this:

/bugzilla 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.

[akhil-rane]

I don't know how to make the bz "target 4.11.0". The only thing I see with "target" is "Target Milestone: ---", which I can't seem to edit. @akhil-rane can you help?

you need to click on show advanced fields and then you target release

[openshift-ci]

@2uasimojo: all tests passed!

Full PR test history. Your PR dashboard.

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.

[codecov]

Codecov Report

Merging #1684 (665b268) into master (1882bbd) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1684   +/-   ##
=======================================
  Coverage   42.34%   42.34%           
=======================================
  Files         337      337           
  Lines       31158    31158           
=======================================
  Hits        13193    13193           
  Misses      16879    16879           
  Partials     1086     1086           

Impacted Files	Coverage Δ
.../openshift/hive/apis/hive/v1/clusterclaim_types.go	100.00% <ø> (ø)
...shift/hive/apis/hive/v1/clusterdeployment_types.go	100.00% <ø> (ø)
...m/openshift/hive/apis/hive/v1/clusterpool_types.go	100.00% <ø> (ø)

[2uasimojo]

Tested live, works. (Though I preferred the native error message.)

The ClusterClaim "the-claim" is invalid: spec.lifetime: Invalid value: "1q": spec.lifetime in body should match '^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$'
...
The ClusterClaim "the-claim" is invalid: spec.lifetime: Invalid value: "1w": spec.lifetime in body should match '^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$'
...
The ClusterClaim "the-claim" is invalid: spec.lifetime: Invalid value: "1d": spec.lifetime in body should match '^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$'
...
(with "1h"):
clusterclaim.hive.openshift.io/the-claim created

And the controllers are happy.

/assign @abutcher

[abutcher]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo, abutcher

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [2uasimojo,abutcher]

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

[openshift-ci]

@2uasimojo: All pull requests linked via external trackers have merged:

• openshift/hive#1684

Bugzilla bug 2050332 has been moved to the MODIFIED state.

In response to this:

Bug 2050332: explicit Pattern to validate Duration fields

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.

[2uasimojo]

@lwan-wanglin how does QE work for things like this with a BZ but no Jira?

[lwan-wanglin]

@lwan-wanglin how does QE work for things like this with a BZ but no Jira?

For hive BZ, we will move Status from ON_QA to Verified after testing. QE behavior won't influnce the hive PR merge because hive won't release within OCP.

For CCO , we now follow non-Feature Freeze workflow, for user stories, we need QE, Docs, and PM add their respective “XXX-approved” label to the PR; for BZ, we need QE add their respective “qe-approved” label.

[2uasimojo]

/cherry-pick ocm-2.5

[openshift-cherrypick-robot]

@2uasimojo: new pull request could not be created: failed to create pull request against openshift/hive#ocm-2.5 from head openshift-cherrypick-robot:cherry-pick-1684-to-ocm-2.5: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"No commits between openshift:ocm-2.5 and openshift-cherrypick-robot:cherry-pick-1684-to-ocm-2.5"}],"documentation_url":"https://docs.github.com/rest/reference/pulls#create-a-pull-request"}

In response to this:

/cherry-pick ocm-2.5

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.

[2uasimojo]

/cherry-pick ocm-2.4

[openshift-cherrypick-robot]

@2uasimojo: #1684 failed to apply on top of branch "ocm-2.4":

Applying: Bug 2050332: explicit Pattern to validate Duration fields
Using index info to reconstruct a base tree...
M	apis/hive/v1/clusterdeployment_types.go
M	apis/hive/v1/clusterpool_types.go
M	config/crds/hive.openshift.io_clusterdeployments.yaml
M	config/crds/hive.openshift.io_clusterpools.yaml
M	hack/app-sre/saas-template.yaml
M	vendor/github.com/openshift/hive/apis/hive/v1/clusterdeployment_types.go
M	vendor/github.com/openshift/hive/apis/hive/v1/clusterpool_types.go
Falling back to patching base and 3-way merge...
Auto-merging vendor/github.com/openshift/hive/apis/hive/v1/clusterpool_types.go
CONFLICT (content): Merge conflict in vendor/github.com/openshift/hive/apis/hive/v1/clusterpool_types.go
Auto-merging vendor/github.com/openshift/hive/apis/hive/v1/clusterdeployment_types.go
Auto-merging hack/app-sre/saas-template.yaml
CONFLICT (content): Merge conflict in hack/app-sre/saas-template.yaml
Auto-merging config/crds/hive.openshift.io_clusterpools.yaml
CONFLICT (content): Merge conflict in config/crds/hive.openshift.io_clusterpools.yaml
Auto-merging config/crds/hive.openshift.io_clusterdeployments.yaml
Auto-merging apis/hive/v1/clusterpool_types.go
CONFLICT (content): Merge conflict in apis/hive/v1/clusterpool_types.go
Auto-merging apis/hive/v1/clusterdeployment_types.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Bug 2050332: explicit Pattern to validate Duration fields
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick ocm-2.4

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.

[2uasimojo]

Manual cherry-pick: #1690