Feature: Add impersonation to tekton results API

[sayan-biswas]

Changes

Resolves #309

This feature adds Kubernetes impersonation header processing, impersonation access check for requester and RBAC for tekton results resources with impersonated user data.

https://kubernetes.io/docs/reference/access-authn-authz/authentication/#user-impersonation

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you review them:

• Has Docs included if any changes are user facing
• Has Tests included if any functionality added or changed
• Tested your changes locally (if this is a code change)
• Follows the commit message standard
• Meets the Tekton contributor standards (including functionality, content, code)
• Has a kind label. You can add a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
• Release notes block below has been updated with any user-facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
• Release notes contain the string "action required" if the change requires additional action from users switching to the new release

Release Notes

Kubernetes impersonation headers can be used when authorizing access to Tekton Results APis.

ACTION REQUIRED: The NO_AUTH config parameter was renamed to AUTH_DISABLE

[tekton-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[sayan-biswas]

/kind feature

[sayan-biswas]

/test all

[sayan-biswas]

/test pull-tekton-results-integration-tests

[sayan-biswas]

/assign @adambkaplan

[adambkaplan]

/approve

Marking this as an approved feature to add. In the current form, however, we need to ensure that e2e tests work with this feature enabled. @dibyom do other projects have a mechanism to test features e2e where the feature flag is enabled and disabled?

[adambkaplan]

Almost there! Code looks good, I added a suggestion to add a code comment.

Can you also please add a section to the Authorization doc that indicates we support Kubernetes impersonation headers [1]? You don't need to explain what user impersonation is - a link to the Kubernetes doc [2] is sufficient. Just focus on how to enable the feature and how Results clients (gRPC, REST) can use it.

[1] https://github.com/tektoncd/results/tree/main/docs/api#authenticationauthorization
[2] https://kubernetes.io/docs/reference/access-authn-authz/authentication/#user-impersonation

[adambkaplan]

/assign @dibyom @alan-ghelardi

There is a breaking configuration change here.

[khrm]

Shouldn't we introduce a deprecation notice instead of directly breaking it? We can remove the NO_AUTH in the next release with the ACTION required notice. In this release, we can have a deprecation notice.

[dibyom]

Shouldn't we introduce a deprecation notice instead of directly breaking it? We can remove the NO_AUTH in the next release with the ACTION required notice. In this release, we can have a deprecation notice.

Technically, we are allowed to make breaking changes in alpha apis. Though generally what we have done in other projects is what @khrm is suggesting here

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adambkaplan, dibyom

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 [adambkaplan,dibyom]

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

[sayan-biswas]

Shouldn't we introduce a deprecation notice instead of directly breaking it? We can remove the NO_AUTH in the next release with the ACTION required notice. In this release, we can have a deprecation notice.

Agreed! But this flag is mostly for development, to bypass the RBAC processing.
Even if this flag is missing from the config, the parses will assume the value as false and continue. Which is the default behaviour anyways.
Also the release will ship the config file which will anyways have the modified parameter.

[sayan-biswas]

Almost there! Code looks good, I added a suggestion to add a code comment.

Can you also please add a section to the Authorization doc that indicates we support Kubernetes impersonation headers [1]? You don't need to explain what user impersonation is - a link to the Kubernetes doc [2] is sufficient. Just focus on how to enable the feature and how Results clients (gRPC, REST) can use it.

[1] https://github.com/tektoncd/results/tree/main/docs/api#authenticationauthorization [2] https://kubernetes.io/docs/reference/access-authn-authz/authentication/#user-impersonation

Added code comment and updated doc.

[adambkaplan]

@alan-ghelardi just want to check with you that renaming the NO_AUTH config setting to AUTH_DISABLE is acceptable (since this feature was your contribution).

Otherwise LGTM.

[adambkaplan]

/lgtm

by lazy consensus.