Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helm: RBAC generation fixes #1461

Merged
merged 4 commits into from
May 21, 2019

Conversation

joelanford
Copy link
Member

Description of the change:

  • Fail e2e test if RBAC generation fails
  • Fix RBAC version parsing to handle OCP versions
  • Return role scaffold immediately upon rule generation failure

Motivation for the change:
This is a follow-on to #1456, which fixed a regression in the Helm RBAC rule generation.

In CI, the RBAC rule generation is currently failing and falling back to the default rule set when using the OCP cluster in Travis. In OCP, the kubernetes version number is reported as 1.11+ (in minikube and upstream kubernetes, it is reported as 1.11). Helm's libraries to generate manifests fail to parse the kubernetes version when it contains the plus sign, so Helm e2e testing has fallen back to the default non-generated rules ever since RBAC generation was added.

The reason that the regression made it through CI is that the changes from #1434 affect only the code path for successful RBAC generation. Since that code path is not running in Travis, it was never tested, and the e2e tests continued to pass because the default non-generated RBAC rules are sufficient for the helm chart used in the e2e tests.

This PR fixes the underlying issue with kubernetes version parsing and improves the e2e test so that future regressions in RBAC rule generation are caught more easily.

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 21, 2019
joelanford added a commit to joelanford/operator-sdk that referenced this pull request May 21, 2019
joelanford added a commit to joelanford/operator-sdk that referenced this pull request May 21, 2019
Copy link
Contributor

@hasbro17 hasbro17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hasbro17 hasbro17 added the kind/bug Categorizes issue or PR as related to a bug. label May 21, 2019
@hasbro17
Copy link
Contributor

This also needs to be backported to the v0.8.x branch right?

@joelanford
Copy link
Member Author

Yes, I think we should backport both. I'll make those PRs after I merge this.

@joelanford joelanford merged commit a67086a into operator-framework:master May 21, 2019
joelanford added a commit to joelanford/operator-sdk that referenced this pull request May 21, 2019
* internal/pkg/scaffold/helm/role.go: return on role generation error

* internal/pkg/scaffold/helm/role.go: handle OCP k8s versions

* hack/tests/e2e-helm.sh: fail on failed RBAC generation

* CHANGELOG.md: added lines for fixes from operator-framework#1456 and operator-framework#1461
joelanford added a commit that referenced this pull request May 21, 2019
* internal/pkg/scaffold/helm/role.go: return on role generation error

* internal/pkg/scaffold/helm/role.go: handle OCP k8s versions

* hack/tests/e2e-helm.sh: fail on failed RBAC generation

* CHANGELOG.md: added lines for fixes from #1456 and #1461
@joelanford joelanford deleted the helm-role-gen-fixes branch May 21, 2019 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants