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

Improve release process and structure of release manifests #3841

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AlanGreene
Copy link
Member

Changes

Reduce unnecessary differences between the read-only and read-write manifests, both to reduce maintenance overhead and to avoid user confusion. There have been a few instances where users thought the read-only mode was broken as they switched the read-only flag in the deployment args but didn't replace the associated roles with the appropriate versions. With this update we now include all roles in the manifest, and create bindings for the relevant ones with updated naming to make it clear which grant view access and which grant edit rather than keeping the same name and swapping out the rules.

Update install script to accept input from a previously generated installer manifest. This removes the need to run the ko build 4 times (2x installer modes + 2x normal release modes) as part of our releases. Now it builds once, and uses the resulting manifest as input to the normal releases so they can augment it with the appropriate RBAC resources depending on mode (read-only vs. read-write). This also means there's no need for a dedicated read-write installer manifest.

Update the release pipeline's publish task to take advantage of this new approach, and eliminate other unnecessary config.

Use the aggregate ClusterRoles provided by Tekton Pipelines and Triggers instead of defining them ourselves, ensuring we stay in sync with the versions deployed on the cluster. The one exception to this is for ClusterTask resources as they're not included in the aggregate roles provided by Tekton Pipelines. We will remove support for these resources in a future release and remove the corresponding rule from the Dashboard backend role.

With this new approach to building the manifests, we no longer need separate overlays or additional patches to modify the resources to match the desired mode. Instead, the installer augments the base installer manifest with the appropriate ClusterRoleBindings or RoleBindings to grant the read-only or read-write permissions as desired, and replaces other config in-place.

Move the remaining resources into the config folder for consistency with other Tekton projects.

All of this should be mostly transparent to consumers. They will still use the installer or release-installer scripts as before. There is some cleanup that should be performed on clusters if upgrading from a previous Dashboard release, by removing the following resources after upgrade:

  • clusterrole/tekton-dashboard-backend
  • clusterrole/tekton-dashboard-tenant
  • clusterrolebinding/tekton-dashboard-backend
  • clusterrolebinding/tekton-dashboard-tenant

and if the Dashboard was installed with limited namespace visibility (i.e. using the --tenant-namespaces installer flag, or directly via the --namespaces deployment arg):

  • rolebinding/tekton-dashboard-tenant in each of the tenant namespaces

/kind misc

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)
  • Has a kind label. You can add one by adding 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 (new features, significant UI changes, API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
  • Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

Renamed and updated a number of ClusterRole and related ClusterRoleBinding or RoleBinding resources for improved clarity and to reduce unnecessary differences between the read-only and read-write installs.

🚨 Action required: If upgrading from a previous Dashboard release, please remove the following resources after upgrade:
- `clusterrole/tekton-dashboard-backend`
- `clusterrole/tekton-dashboard-tenant`
- `clusterrolebinding/tekton-dashboard-backend`
- `clusterrolebinding/tekton-dashboard-tenant`

and if the Dashboard was installed with limited namespace visibility (i.e. using the `--tenant-namespaces` installer flag, or directly via the `--namespaces` deployment arg):
- `rolebinding/tekton-dashboard-tenant` in each of the tenant namespaces

These are no longer included in the install and should be removed to avoid unexpected behaviour or inadvertently granting unexpected permissions to the Dashboard's `ServiceAccount`.

@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from alangreene after the PR has been reviewed.

The full list of commands accepted by this bot can be found 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 kind/misc Categorizes issue or PR as a miscellaneuous one. label Dec 13, 2024
@tekton-robot tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Dec 13, 2024
@AlanGreene AlanGreene removed the request for review from skaegi December 13, 2024 00:18
Reduce unnecessary differences between the read-only and read-write manifests,
both to reduce maintenance overhead and to avoid user confusion. There have
been a few instances where users thought the read-only mode was broken as they
switched the `read-only` flag in the deployment args but didn't replace the
associated roles with the appropriate versions. With this update we now include
all roles in the manifest, and create bindings for the relevant ones with
updated naming to make it clear which grant view access and which grant edit
rather than keeping the same name and swapping out the rules.

Update install script to accept input from a previously generated installer
manifest. This removes the need to run the `ko` build 4 times (2x installer
modes + 2x normal release modes) as part of our releases. Now it builds once,
and uses the resulting manifest as input to the normal releases so they can
augment it with the appropriate RBAC resources depending on mode (read-only vs.
read-write). This also means there's no need for a dedicated read-write
installer manifest.

Update the release pipeline's publish task to take advantage of this new
approach, and eliminate other unnecessary config.

Use the aggregate ClusterRoles provided by Tekton Pipelines and Triggers
instead of defining them ourselves, ensuring we stay in sync with the
versions deployed on the cluster. The one exception to this is for
`ClusterTask` resources as they're not included in the aggregate roles
provided by Tekton Pipelines. We will remove support for these resources
in a future release and remove the corresponding rule from the Dashboard
backend role.

With this new approach to building the manifests, we no longer need
separate overlays or additional patches to modify the resources to match
the desired mode. Instead, the installer augments the base installer
manifest with the appropriate ClusterRoleBindings or RoleBindings to grant
the read-only or read-write permissions as desired, and replaces other
config in-place.

Move the remaining resources into the `config` folder for consistency
with other Tekton projects.

All of this should be mostly transparent to consumers. They will still use
the `installer` or `release-installer` scripts as before. There is some
cleanup that should be performed on clusters if upgrading from a previous
Dashboard release, by removing the following resources after upgrade:
- `clusterrole/tekton-dashboard-backend`
- `clusterrole/tekton-dashboard-tenant`
- `clusterrolebinding/tekton-dashboard-backend`
- `clusterrolebinding/tekton-dashboard-tenant`

and if the Dashboard was installed with limited namespace visibility
(i.e. using the `--tenant-namespaces` installer flag, or directly via the
`--namespaces` deployment arg):
- `rolebinding/tekton-dashboard-tenant` in each of the tenant namespaces
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/misc Categorizes issue or PR as a miscellaneuous one. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants