Exception for console extension CRDs in extended tests #23231

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.

Sign up for GitHub

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

Jump to bottom

Merged

openshift-merge-robot merged 1 commit into openshift:master from benjaminapetersen:console/rbac/group-rules/crd-exceptions

Jun 21, 2019

+7 −1

Contributor

benjaminapetersen commented Jun 20, 2019

Add system:authenticated exception for CRDs used by console for extensions

These 3 CRDs:

consoleclidownloads
consolelinks
consolenotifications

Are intended to be used by other operators to enhance the console experience. As such, they can be requested by any logged in user. For example, A cluster admin could install an operator that wants to add additional links to the console menu. Users of the console should be able to access these links to use the new functionality.

The console team is exploring ways to instead proxy these requests through the console backend, using the console Service Account token, instead of needing special bindings to system:authenticated.

Discussion regarding the reasonableness of this exception:

Add console extension manifests console-operator#229 (comment)

Related PR adding the CRDs to the Console Operator:

Add console extension manifests console-operator#229

Related PRs adding the features to the Console:

/assign @spadgett @enj

openshift-ci-robot assigned enj and spadgett

openshift-ci-robot added the size/XS label

openshift-ci-robot requested review from enj and ericavonb

June 20, 2019 18:13

benjaminapetersen force-pushed the console/rbac/group-rules/crd-exceptions branch 2 times, most recently from c56a6d5 to 1b8ed72 Compare

June 20, 2019 18:26

Member

spadgett commented Jun 20, 2019

openshift-ci-robot requested a review from bparees

June 20, 2019 18:26

Contributor

enj commented Jun 20, 2019

/assign @bparees

Changes are fine. Ben gets to tag.

openshift-ci-robot assigned bparees

Contributor

bparees commented Jun 20, 2019

/lgtm

openshift-ci-robot added lgtm approved labels

benjaminapetersen mentioned this pull request

[WIP] Add ConsoleLogLink CRD to enable custom log links openshift/console#1776

Closed

Contributor

openshift-bot commented Jun 20, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

10 similar comments

Contributor

openshift-bot commented Jun 20, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 20, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 21, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 21, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 21, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 21, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 21, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 21, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 21, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 21, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

Member

spadgett commented Jun 21, 2019

/hold

fail [github.com/openshift/origin/test/extended/authorization/rbac/groups_default_rules.go:207]: Jun 21 11:46:08.080: test data for system:authenticated has too many unnecessary permissions: {APIGroups:["console.openshift.io"], Resources:["consoleclidownloads"], Verbs:["get" "list" "watch"]} {APIGroups:["console.openshift.io"], Resources:["consolelinks"], Verbs:["get" "list" "watch"]} {APIGroups:["console.openshift.io"], Resources:["consolenotifications"], Verbs:["get" "list" "watch"]}

openshift-ci-robot added the do-not-merge/hold label

Contributor

enj commented Jun 21, 2019

lol change the 5 to a 10. The new rules expanded to be simple rules is likely 9 items long.


          Add system:authenticated exception for CRDs used by console for exten…

bb09b26

…sions

benjaminapetersen force-pushed the console/rbac/group-rules/crd-exceptions branch from 1b8ed72 to bb09b26 Compare

June 21, 2019 13:40

openshift-ci-robot removed the lgtm label

Contributor Author

benjaminapetersen commented Jun 21, 2019

Bumped the limit. See if it passes by tomorrow afternoon :)

Member

spadgett commented Jun 21, 2019

/hold cancel

openshift-ci-robot removed the do-not-merge/hold label

spadgett approved these changes

View reviewed changes

Member

spadgett left a comment

/lgtm

openshift-ci-robot added the lgtm label

openshift-ci-robot commented Jun 21, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benjaminapetersen, bparees, spadgett

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:

~~test/extended/authorization/rbac/OWNERS~~ [bparees]

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

openshift-merge-robot merged commit ed37c6c into openshift:master

This was referenced Jul 15, 2019

Add console extension manfiest for ConsoleExternalLogLinks CRD openshift/console-operator#246

Merged

Add ConsoleLogLinks to exceptions list for Console CRDs #23382

Merged

benjaminapetersen deleted the console/rbac/group-rules/crd-exceptions branch

October 15, 2019 20:28

benjaminapetersen mentioned this pull request

Add consoleyamlsamples to list of console resource exceptions #23975

Merged

wking added a commit to wking/origin that referenced this pull request


          test/extended/authorization/rbac: Condition console RBAC on 'Console'…

3d67ac6

… capability

Clusters that disable the 'Console' capability are currently failing
this test-case [1]:

  : [sig-auth][Feature:OpenShiftAuthorization] The default cluster RBAC policy should have correct RBAC rules [Suite:openshift/conformance/parallel] expand_less
  Run #0: Failed expand_less	3s
  {  fail [github.com/openshift/origin/test/extended/authorization/rbac/groups_default_rules.go:229]: Jan  3 13:43:14.134: test data for system:authenticated has too many unnecessary permissions:
  {APIGroups:["console.openshift.io"], Resources:["consoleclidownloads"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleexternalloglinks"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolelinks"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolenotifications"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleplugins"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolequickstarts"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleyamlsamples"], Verbs:["get" "list" "watch"]}
  {APIGroups:["helm.openshift.io"], Resources:["helmchartrepositories"], Verbs:["get" "list"]}
  {APIGroups:["snapshot.storage.k8s.io"], Resources:["volumesnapshotclasses"], Verbs:["get" "list" "watch"]}
  Ginkgo exit error 1: exit with code 1}

This commit uses the pattern that 6d1143a (cli: remove metal3 CRDs
when capabilities are none, 2022-04-08, openshift#26998) began using for CRDs
to only add the console-linked rules when Console is enabled.

It has also been around three years since bb09b26 (Add
system:authenticated exception for CRDs used by console for
extensions, 2019-06-21, openshift#23231)'s "eliminating this exception in the
near future", so I'm softening that to "may eventually".  Extending
system:authenticated is still not a great pattern to follow, but it
may never be worth the time it would take the console team to build an
alternative mechanism.

[1]: https://prow.ci.openshift.org/view/gs/origin-ci-test/logs/periodic-ci-openshift-release-master-ci-4.13-e2e-aws-sdn-no-capabilities/1610257913278894080

wking added a commit to wking/origin that referenced this pull request


          test/extended/authorization/rbac: Condition console RBAC on 'Console'…

daa1550

… capability

Clusters that disable the 'Console' capability are currently failing
this test-case [1]:

  : [sig-auth][Feature:OpenShiftAuthorization] The default cluster RBAC policy should have correct RBAC rules [Suite:openshift/conformance/parallel] expand_less
  Run #0: Failed expand_less	3s
  {  fail [github.com/openshift/origin/test/extended/authorization/rbac/groups_default_rules.go:229]: Jan  3 13:43:14.134: test data for system:authenticated has too many unnecessary permissions:
  {APIGroups:["console.openshift.io"], Resources:["consoleclidownloads"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleexternalloglinks"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolelinks"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolenotifications"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleplugins"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolequickstarts"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleyamlsamples"], Verbs:["get" "list" "watch"]}
  {APIGroups:["helm.openshift.io"], Resources:["helmchartrepositories"], Verbs:["get" "list"]}
  {APIGroups:["snapshot.storage.k8s.io"], Resources:["volumesnapshotclasses"], Verbs:["get" "list" "watch"]}
  Ginkgo exit error 1: exit with code 1}

This commit uses the pattern that 6d1143a (cli: remove metal3 CRDs
when capabilities are none, 2022-04-08, openshift#26998) began using for CRDs
to only add the console-linked rules when Console is enabled, and to
only add the snapshot-linked rules when `CSISnapshot` is enabled.

It has also been around three years since bb09b26 (Add
system:authenticated exception for CRDs used by console for
extensions, 2019-06-21, openshift#23231)'s "eliminating this exception in the
near future", so I'm softening that to "may eventually".  Extending
system:authenticated is still not a great pattern to follow, but it
may never be worth the time it would take the console team to build an
alternative mechanism.

[1]: https://prow.ci.openshift.org/view/gs/origin-ci-test/logs/periodic-ci-openshift-release-master-ci-4.13-e2e-aws-sdn-no-capabilities/1610257913278894080

wking added a commit to wking/origin that referenced this pull request


          test/extended/authorization/rbac: Condition console RBAC on 'Console'…

fa64951

… capability

Clusters that disable the 'Console' capability are currently failing
this test-case [1]:

  : [sig-auth][Feature:OpenShiftAuthorization] The default cluster RBAC policy should have correct RBAC rules [Suite:openshift/conformance/parallel] expand_less
  Run #0: Failed expand_less	3s
  {  fail [github.com/openshift/origin/test/extended/authorization/rbac/groups_default_rules.go:229]: Jan  3 13:43:14.134: test data for system:authenticated has too many unnecessary permissions:
  {APIGroups:["console.openshift.io"], Resources:["consoleclidownloads"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleexternalloglinks"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolelinks"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolenotifications"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleplugins"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolequickstarts"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleyamlsamples"], Verbs:["get" "list" "watch"]}
  {APIGroups:["helm.openshift.io"], Resources:["helmchartrepositories"], Verbs:["get" "list"]}
  {APIGroups:["snapshot.storage.k8s.io"], Resources:["volumesnapshotclasses"], Verbs:["get" "list" "watch"]}
  Ginkgo exit error 1: exit with code 1}

This commit uses the pattern that 6d1143a (cli: remove metal3 CRDs
when capabilities are none, 2022-04-08, openshift#26998) began using for CRDs
to only add the console-linked rules when Console is enabled, and to
only add the snapshot-linked rules when `CSISnapshot` is enabled.

It has also been around three years since bb09b26 (Add
system:authenticated exception for CRDs used by console for
extensions, 2019-06-21, openshift#23231)'s "eliminating this exception in the
near future", so I'm softening that to "may eventually".  Extending
system:authenticated is still not a great pattern to follow, but it
may never be worth the time it would take the console team to build an
alternative mechanism.

[1]: https://prow.ci.openshift.org/view/gs/origin-ci-test/logs/periodic-ci-openshift-release-master-ci-4.13-e2e-aws-sdn-no-capabilities/1610257913278894080

wking added a commit to wking/origin that referenced this pull request


          test/extended/authorization/rbac: Condition console RBAC on 'Console'…

61fdce8

… capability

Clusters that disable the 'Console' capability are currently failing
this test-case [1]:

  : [sig-auth][Feature:OpenShiftAuthorization] The default cluster RBAC policy should have correct RBAC rules [Suite:openshift/conformance/parallel] expand_less
  Run #0: Failed expand_less	3s
  {  fail [github.com/openshift/origin/test/extended/authorization/rbac/groups_default_rules.go:229]: Jan  3 13:43:14.134: test data for system:authenticated has too many unnecessary permissions:
  {APIGroups:["console.openshift.io"], Resources:["consoleclidownloads"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleexternalloglinks"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolelinks"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolenotifications"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleplugins"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolequickstarts"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleyamlsamples"], Verbs:["get" "list" "watch"]}
  {APIGroups:["helm.openshift.io"], Resources:["helmchartrepositories"], Verbs:["get" "list"]}
  {APIGroups:["snapshot.storage.k8s.io"], Resources:["volumesnapshotclasses"], Verbs:["get" "list" "watch"]}
  Ginkgo exit error 1: exit with code 1}

This commit uses the pattern that 6d1143a (cli: remove metal3 CRDs
when capabilities are none, 2022-04-08, openshift#26998) began using for CRDs
to only add the console-linked rules when Console is enabled, and to
only add the snapshot-linked rules when `CSISnapshot` is enabled.

It has also been around three years since bb09b26 (Add
system:authenticated exception for CRDs used by console for
extensions, 2019-06-21, openshift#23231)'s "eliminating this exception in the
near future", so I'm softening that to "may eventually".  Extending
system:authenticated is still not a great pattern to follow, but it
may never be worth the time it would take the console team to build an
alternative mechanism.

[1]: https://prow.ci.openshift.org/view/gs/origin-ci-test/logs/periodic-ci-openshift-release-master-ci-4.13-e2e-aws-sdn-no-capabilities/1610257913278894080

wking added a commit to wking/origin that referenced this pull request


          test/extended/authorization/rbac: Condition console RBAC on 'Console'…

f655a70

… capability

Clusters that disable the 'Console' capability are currently failing
this test-case [1]:

  : [sig-auth][Feature:OpenShiftAuthorization] The default cluster RBAC policy should have correct RBAC rules [Suite:openshift/conformance/parallel] expand_less
  Run #0: Failed expand_less	3s
  {  fail [github.com/openshift/origin/test/extended/authorization/rbac/groups_default_rules.go:229]: Jan  3 13:43:14.134: test data for system:authenticated has too many unnecessary permissions:
  {APIGroups:["console.openshift.io"], Resources:["consoleclidownloads"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleexternalloglinks"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolelinks"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolenotifications"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleplugins"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolequickstarts"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleyamlsamples"], Verbs:["get" "list" "watch"]}
  {APIGroups:["helm.openshift.io"], Resources:["helmchartrepositories"], Verbs:["get" "list"]}
  {APIGroups:["snapshot.storage.k8s.io"], Resources:["volumesnapshotclasses"], Verbs:["get" "list" "watch"]}
  Ginkgo exit error 1: exit with code 1}

This commit uses the pattern that 6d1143a (cli: remove metal3 CRDs
when capabilities are none, 2022-04-08, openshift#26998) began using for CRDs
to only add the console-linked rules when Console is enabled, and to
only add the snapshot-linked rules when `CSISnapshot` is enabled.

MicroShift won't have a ClusterVersion custom resource definition, but
the test is already failing there [2], so this pivot doesn't break
them any worse.  Once they have a plan for how they would like to
handle it, they can come back and make those changes in follow-up
work.

It has also been around three years since bb09b26 (Add
system:authenticated exception for CRDs used by console for
extensions, 2019-06-21, openshift#23231)'s "eliminating this exception in the
near future", so I'm softening that to "may eventually".  Extending
system:authenticated is still not a great pattern to follow, but it
may never be worth the time it would take the console team to build an
alternative mechanism.

[1]: https://prow.ci.openshift.org/view/gs/origin-ci-test/logs/periodic-ci-openshift-release-master-ci-4.13-e2e-aws-sdn-no-capabilities/1610257913278894080
[2]: openshift#27681 (comment)

tjungblu pushed a commit to tjungblu/origin that referenced this pull request


          test/extended/authorization/rbac: Condition console RBAC on 'Console'…

f47073a

… capability

Clusters that disable the 'Console' capability are currently failing
this test-case [1]:

  : [sig-auth][Feature:OpenShiftAuthorization] The default cluster RBAC policy should have correct RBAC rules [Suite:openshift/conformance/parallel] expand_less
  Run #0: Failed expand_less	3s
  {  fail [github.com/openshift/origin/test/extended/authorization/rbac/groups_default_rules.go:229]: Jan  3 13:43:14.134: test data for system:authenticated has too many unnecessary permissions:
  {APIGroups:["console.openshift.io"], Resources:["consoleclidownloads"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleexternalloglinks"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolelinks"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolenotifications"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleplugins"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consolequickstarts"], Verbs:["get" "list" "watch"]}
  {APIGroups:["console.openshift.io"], Resources:["consoleyamlsamples"], Verbs:["get" "list" "watch"]}
  {APIGroups:["helm.openshift.io"], Resources:["helmchartrepositories"], Verbs:["get" "list"]}
  {APIGroups:["snapshot.storage.k8s.io"], Resources:["volumesnapshotclasses"], Verbs:["get" "list" "watch"]}
  Ginkgo exit error 1: exit with code 1}

This commit uses the pattern that 6d1143a (cli: remove metal3 CRDs
when capabilities are none, 2022-04-08, openshift#26998) began using for CRDs
to only add the console-linked rules when Console is enabled, and to
only add the snapshot-linked rules when `CSISnapshot` is enabled.

MicroShift won't have a ClusterVersion custom resource definition, but
the test is already failing there [2], so this pivot doesn't break
them any worse.  Once they have a plan for how they would like to
handle it, they can come back and make those changes in follow-up
work.

It has also been around three years since bb09b26 (Add
system:authenticated exception for CRDs used by console for
extensions, 2019-06-21, openshift#23231)'s "eliminating this exception in the
near future", so I'm softening that to "may eventually".  Extending
system:authenticated is still not a great pattern to follow, but it
may never be worth the time it would take the console team to build an
alternative mechanism.

[1]: https://prow.ci.openshift.org/view/gs/origin-ci-test/logs/periodic-ci-openshift-release-master-ci-4.13-e2e-aws-sdn-no-capabilities/1610257913278894080
[2]: openshift#27681 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm size/XS