OSD-8714 - Automate STS diff in osdctl#155
OSD-8714 - Automate STS diff in osdctl#155openshift-merge-robot merged 1 commit intoopenshift:masterfrom
Conversation
There was a problem hiding this comment.
This is great. Minor issue: when passing a version with only x.y (i.e. 4.9) the command fails with exit status 1error: exit status 1 (formatting issue) and leaves the /tmp/crs-4.9 behind. Since the images will use x.y.z, the arguments should be validated to ensure all three numbers are present, with failure (and documentation) showing the correct formatting of the version. Also the /tmp/crs- directory should be removed if the command errors out if possible.
|
Thank you, @fahlmant, for catching this issue. I have fixed the code for both of your concerns. |
|
@Tessg22 Looks great! Mind squashing? |
cmd/cluster/policy.go
Outdated
| return cmdutil.UsageErrorf(cmd, "Release version is required for policy command") | ||
| } | ||
|
|
||
| re := regexp.MustCompile(`^[0-9]{1}\.[0-9]{1}\.[0-9]{1,2}$`) |
There was a problem hiding this comment.
This regex would have issues once we get to 4.10 I think (2-digit y-streams)
Maybe overkill for this task, but semver is an awesome package for verifying versions.
cmd/cluster/policydiff.go
Outdated
| } | ||
|
|
||
| for _, s := range args { | ||
| re := regexp.MustCompile(`^[0-9]{1}\.[0-9]{1}\.[0-9]{1,2}$`) |
There was a problem hiding this comment.
Same comment here as per elsewhere.
|
@Tessg22 One more question: Why is this being placed under the |
|
@fahlmant I see your point. Do you think |
|
@Tessg22 Yes that looks good to me! |
docs(): add section for policy and policy-diff commands fix(): add version validation refactor(): add parent sts command and fix regex version validation
Tessg22
force-pushed
the
add-sts-policy-diff
branch
from
f56a0b2 to
9bb8c50
Compare
fahlmant
left a comment
fahlmant
left a comment
There was a problem hiding this comment.
/lgtm
/approve
This is awesome! Thanks for the work! :)
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fahlmant, Tessg22 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
OSD-8714 - Automate STS diff in osdctl
4 participants
OSDCTL can output diff between two versions of OCP STS policy or the current permission set of a version of OCP.
Resolves: https://issues.redhat.com/browse/OSD-8714