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

Add ConfigMap which can contain pipelines info and RBAC to access this ConfigMap #3971

Merged
merged 1 commit into from Jun 1, 2021

Conversation

vinamra28
Copy link
Member

Changes

As of now we fetch version of pipelines through labels present on the
deployments which is read by tools such as tkn cli and display the
version. This version may not be displayed to users if they don't have
permission to view the deployment.

In this commit we are adding

  1. A ConfigMap which contains version information.
  2. RBAC which will give appropriate permissions to view the ConfigMap
    irrespective of whether user is has permission to view other objects in
    that namespace or not.

This is covered as part of TEP-0041

Signed-off-by: vinamra28 vinjain@redhat.com

/kind feature

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Docs included if any changes are user facing
  • Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including
    functionality, content, code)
  • Release notes block below has been filled in or deleted (only if no user facing changes)

Release Notes

Add ConfigMap which will contains Pipelines info such as version and some RBAC rules which will give access to this ConfigMap to all the system:authenticated users.

@tekton-robot tekton-robot added kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels May 24, 2021
@tekton-robot tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 24, 2021
@vinamra28 vinamra28 force-pushed the implement-tep-0041 branch 3 times, most recently from a5e165f to 2374d1a Compare May 24, 2021 04:06
@ghost
Copy link

ghost commented May 25, 2021

It would be great to include some comments in the ConfigMap and the role / rolebinding giving a short explanation of the purpose. "allow tools to query for the current installed pipelines version without elevated permissions" or something similar, wdyt?

As of now we fetch version of pipelines through labels present on the
deployments which is read by tools such as `tkn cli` and display the
version. This version may not be displayed to users if they don't have
permission to view the deployment.

In this commit we are adding
1. A `ConfigMap` which contains version information.
2. RBAC which will give appropriate permissions to view the ConfigMap
irrespective of whether user is has permission to view other objects in
that namespace or not.

Signed-off-by: vinamra28 <vinjain@redhat.com>
@vinamra28
Copy link
Member Author

/retest

@vinamra28
Copy link
Member Author

/test check-pr-has-kind-label

@nikhil-thomas
Copy link
Member

looks like check-pr-has-kind-label pipelinerun is passing, but the result is not updated here.
i noticed the same behavior yesterday as well.

cc @afrittoli

Screenshot 2021-05-26 at 12 19 48 PM

@ghost ghost removed the kind/feature Categorizes issue or PR as related to a new feature. label May 26, 2021
@ghost
Copy link

ghost commented May 26, 2021

/kind feature

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label May 26, 2021
@ghost
Copy link

ghost commented May 26, 2021

How will the ConfigMap be updated with the release version? Is the idea that the operator will take care of this or we'll bake it into the release.yaml somehow during the pipelines release process?

@vinamra28
Copy link
Member Author

How will the ConfigMap be updated with the release version? Is the idea that the operator will take care of this or we'll bake it into the release.yaml somehow during the pipelines release process?

@sbwsg yeah during the release process the value will get updated with the help of following sed command and this ConfigMap will become the part of release.yaml

sed -i -e 's/\(pipeline.tekton.dev\/release\): "devel"/\1: "$(params.versionTag)"/g' -e 's/\(app.kubernetes.io\/version\): "devel"/\1: "$(params.versionTag)"/g' -e 's/\(version\): "devel"/\1: "$(params.versionTag)"/g' -e 's/\("-version"\), "devel"/\1, "$(params.versionTag)"/g' ${PROJECT_ROOT}/config/*.yaml

@ghost
Copy link

ghost commented May 26, 2021

/approve

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbwsg

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:

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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 26, 2021
@nikhil-thomas
Copy link
Member

looks good 👍

@vinamra28
Copy link
Member Author

/cc @afrittoli @imjasonh

@vinamra28
Copy link
Member Author

/cc @vdemeester

@vdemeester
Copy link
Member

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 1, 2021
@vinamra28
Copy link
Member Author

/retest

@vinamra28
Copy link
Member Author

/retest

@vinamra28
Copy link
Member Author

Build tests ran successfully but prow didn't reported it 🤔
integration-alpha-tests didn't ran completely 😅
/retest

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. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants