Skip to content

Run conformance tests as project admin#5596

Merged
knative-prow-robot merged 12 commits into
knative:mainfrom
mgencur:project_admin_conformance_main
Aug 2, 2021
Merged

Run conformance tests as project admin#5596
knative-prow-robot merged 12 commits into
knative:mainfrom
mgencur:project_admin_conformance_main

Conversation

@mgencur
Copy link
Copy Markdown
Contributor

@mgencur mgencur commented Jul 16, 2021

This partially fixes #5357 but the user is able to run only conformance tests so far.
With the current changes, and by following the readme below, there are 48 tests passing in the conformance test suite.

Proposed Changes

  • move RBAC creation to a common place
  • use RBAC that is named according to the test namespace (one set for event logger, and one set for tests using ApiServerSource)
  • add !project_admin build tag to tests that require cluster-admin permissions (this is mostly because they manipulate CRDs or read configs from knative-eventing namespace and try to do port-forwarding to a pod in Knative Eventing namespace (Tracing))
  • helper script for creating NS, SA, Role, RoleBinding
  • instructions in readme
  • remove unused CreateRBACResourcesForBrokers function

Just for reference, we already have similar instructions in Knative Serving and Knative Client:

This PR doesn't include:

  • changes to REKT test suite
  • changes to E2E test suite - not sure it's worth as this one requires cluster-admin privileges in most tests

Pre-review Checklist

  • At least 80% unit test coverage
  • E2E tests for any new behavior
  • Docs PR for any user-facing impact
  • Spec PR for any new API feature
  • Conformance test for any change to the spec

Release Note


Docs

mgencur added 5 commits July 16, 2021 15:47
* most tests will use just the serviceAccount that has the same name as
the test namespace
* tests using ApiServerSource will user serviceAccount that is named as
follows: ${namespace}-eventwatcher
@knative-prow-robot knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/test-and-release Test infrastructure, tests or release labels Jul 16, 2021
@mgencur mgencur changed the title Run conformance tests by project admin user Run conformance tests as project admin Jul 16, 2021
@google-cla google-cla Bot added the cla: yes Indicates the PR's author has signed the CLA. label Jul 16, 2021
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 16, 2021

Codecov Report

Merging #5596 (513143c) into main (25bd8ef) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #5596   +/-   ##
=======================================
  Coverage   82.79%   82.79%           
=======================================
  Files         199      199           
  Lines        6230     6230           
=======================================
  Hits         5158     5158           
  Misses        744      744           
  Partials      328      328           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 25bd8ef...513143c. Read the comment docs.

@mgencur mgencur force-pushed the project_admin_conformance_main branch from aa64eed to 98ccb69 Compare July 16, 2021 14:08
@mgencur mgencur changed the title Run conformance tests as project admin [WIP] Run conformance tests as project admin Jul 19, 2021
@knative-prow-robot knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 19, 2021
mgencur added 5 commits July 19, 2021 09:47
* since the name of the resources is aligned with the name of the
namespace it makes sense to create them only once, at the same time as
the namespace
@mgencur mgencur changed the title [WIP] Run conformance tests as project admin Run conformance tests as project admin Jul 19, 2021
@knative-prow-robot knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 19, 2021
@lionelvillard
Copy link
Copy Markdown
Contributor

@aliok can you TAL at this PR?

Copy link
Copy Markdown
Member

@matzew matzew left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 2, 2021
@knative-prow-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matzew, mgencur

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:

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

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 2, 2021
@knative-prow-robot knative-prow-robot merged commit a8a3063 into knative:main Aug 2, 2021
mgencur added a commit to mgencur/eventing that referenced this pull request Aug 3, 2021
* Create RBAC according to the test namespace

* most tests will use just the serviceAccount that has the same name as
the test namespace
* tests using ApiServerSource will user serviceAccount that is named as
follows: ${namespace}-eventwatcher

* Script for creating necessary RBAC for conformance tests

* Readme for running Conformance tests as project admin

* Fix imports

* Minor update for readme

* Fix goimport

* Do not fail if ServiceAccount already exists

* Fix goftm

* Mark tests requiring cluster-admin with specific build tag

* Move the creation of SA,Role,RoleBinding close to namespace creation

* since the name of the resources is aligned with the name of the
namespace it makes sense to create them only once, at the same time as
the namespace

* Formatting

* Move remaining creation of SA,Role,RoleBinding
mgencur added a commit to mgencur/eventing that referenced this pull request Aug 3, 2021
* Create RBAC according to the test namespace

* most tests will use just the serviceAccount that has the same name as
the test namespace
* tests using ApiServerSource will user serviceAccount that is named as
follows: ${namespace}-eventwatcher

* Script for creating necessary RBAC for conformance tests

* Readme for running Conformance tests as project admin

* Fix imports

* Minor update for readme

* Fix goimport

* Do not fail if ServiceAccount already exists

* Fix goftm

* Mark tests requiring cluster-admin with specific build tag

* Move the creation of SA,Role,RoleBinding close to namespace creation

* since the name of the resources is aligned with the name of the
namespace it makes sense to create them only once, at the same time as
the namespace

* Formatting

* Move remaining creation of SA,Role,RoleBinding
openshift-ci Bot pushed a commit to openshift/knative-eventing that referenced this pull request Aug 4, 2021
* Create RBAC according to the test namespace

* most tests will use just the serviceAccount that has the same name as
the test namespace
* tests using ApiServerSource will user serviceAccount that is named as
follows: ${namespace}-eventwatcher

* Script for creating necessary RBAC for conformance tests

* Readme for running Conformance tests as project admin

* Fix imports

* Minor update for readme

* Fix goimport

* Do not fail if ServiceAccount already exists

* Fix goftm

* Mark tests requiring cluster-admin with specific build tag

* Move the creation of SA,Role,RoleBinding close to namespace creation

* since the name of the resources is aligned with the name of the
namespace it makes sense to create them only once, at the same time as
the namespace

* Formatting

* Move remaining creation of SA,Role,RoleBinding
openshift-ci Bot pushed a commit to openshift/knative-eventing that referenced this pull request Aug 5, 2021
* Use test namespaces with numeric suffix (knative#5337)

* Use test namespaces from pre-defined range

* ReuseNamespace flag

* ReuseNamespace variable in lib package

* prevents import cycle when flags are accessed directly from the lib
package

* Fix gofmt

* Fix goimports

* Remove the last occurrence of import cycle

* Remove unused variable

* Fix detection of NotFound api error

* also make the retry count and interval shorter to speed up things

* Delete NS only if we created it

* leave previously existing namespaces there

* Call CreateNamespaceWithRetry directly

* Run conformance tests as project admin (knative#5596)

* Create RBAC according to the test namespace

* most tests will use just the serviceAccount that has the same name as
the test namespace
* tests using ApiServerSource will user serviceAccount that is named as
follows: ${namespace}-eventwatcher

* Script for creating necessary RBAC for conformance tests

* Readme for running Conformance tests as project admin

* Fix imports

* Minor update for readme

* Fix goimport

* Do not fail if ServiceAccount already exists

* Fix goftm

* Mark tests requiring cluster-admin with specific build tag

* Move the creation of SA,Role,RoleBinding close to namespace creation

* since the name of the resources is aligned with the name of the
namespace it makes sense to create them only once, at the same time as
the namespace

* Formatting

* Move remaining creation of SA,Role,RoleBinding
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release Test infrastructure, tests or release cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Introduce a way how to run the e2e TS without admin permissions

4 participants