Add readonly property to Tekton Dashboard component #402

MarcusNoble · 2021-09-06T07:20:44Z

Changes

Fixes #399

Adds an optional boolean property to the TektonDashboard and the TektonConfig CRDs to control which variation of the Dashboard manifest is used.
If readonly is set to true the manifests with only the read RBAC permissions will be used.

Submitter Checklist

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

Includes tests (if functionality changed/added)
Includes docs (if user facing)
Commit messages follow commit message best practices

See the contribution guide for more details.

Release Notes

Added `readonly` property to TektonDashboard to allow installing the Dashboard in read-only mode

tekton-robot · 2021-09-06T07:20:58Z

Hi @MarcusNoble. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

nikhil-thomas · 2021-09-06T08:04:48Z

/ok-to-test

nikhil-thomas · 2021-09-06T12:15:10Z

hack/fetch-releases.sh

+    if [[ ${releaseFileName} == "tekton-dashboard-release-readonly" ]]; then
+      dir="dashboard-readonly"


could we create sub-directories in dashboard dir instead of creating a new directory.
I would prefer that becuase operator-dev has an informal agreement that we keep 1 top level directory per TektonComponent. 🧑‍💻

nikhil-thomas · 2021-09-06T12:17:49Z

overall the pr looks great.
It would be great if we could keep all dashboard related items in subdirectories inside a top level directory like kodata/dashboard.

nikhil-thomas · 2021-09-06T12:19:20Z

pkg/reconciler/common/releases.go

+			return filepath.Join(koDataDir, "tekton-dashboard-readonly")
+		}
 		return filepath.Join(koDataDir, "tekton-dashboard")


How about:

Suggested change

return filepath.Join(koDataDir, "tekton-dashboard-readonly")

}

return filepath.Join(koDataDir, "tekton-dashboard")

return filepath.Join(koDataDir, "tekton-dashboard/tekton-dashboard-readonly")

}

return filepath.Join(koDataDir, "tekton-dashboard/tekton-dashboard-fullaccess")

nikhil-thomas · 2021-09-06T12:19:42Z

pkg/reconciler/common/releases.go

+			return filepath.Join(koDataDir, "tekton-dashboard-readonly")
+		}
 		return filepath.Join(koDataDir, "tekton-dashboard")


How about:

Suggested change

return filepath.Join(koDataDir, "tekton-dashboard-readonly")

}

return filepath.Join(koDataDir, "tekton-dashboard")

return filepath.Join(koDataDir, "tekton-dashboard/tekton-dashboard-readonly")

}

return filepath.Join(koDataDir, "tekton-dashboard/tekton-dashboard-fullaccess")

MarcusNoble · 2021-09-06T12:53:58Z

Thanks @nikhil-thomas. I've updated with those suggestions.

nikhil-thomas · 2021-09-08T00:55:14Z

/ok-to-test

nikhil-thomas · 2021-09-08T00:55:23Z

/approve

tekton-robot · 2021-09-08T00:58:42Z

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/common/releases.go	63.6%	61.8%	-1.9
pkg/reconciler/kubernetes/tektonconfig/extension/dashboard.go	58.6%	55.7%	-2.9

MarcusNoble · 2021-09-09T05:58:28Z

I'm having a bit of trouble running the hack/update-codegen.sh for this PR

# hack/update-codegen.sh  
Generating deepcopy funcs for operator:v1alpha1
Generating clientset for operator:v1alpha1 at github.com/tektoncd/operator/pkg/client/clientset
Generating listers for operator:v1alpha1 at github.com/tektoncd/operator/pkg/client/listers
Generating informers for operator:v1alpha1 at github.com/tektoncd/operator/pkg/client/informers
Generating injection for operator:v1alpha1 at github.com/tektoncd/operator/pkg/client/injection
go: downloading go.uber.org/automaxprocs v1.4.0
go: downloading go.uber.org/atomic v1.9.0
go: downloading github.com/googleapis/gnostic v0.5.3
go: downloading golang.org/x/time v0.0.0-20210611083556-38a9dc6acbc6
go: downloading golang.org/x/net v0.0.0-20210825183410-e898025ed96a
go: downloading golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f
go: downloading google.golang.org/grpc v1.40.0
verifying github.com/googleapis/[email protected]: checksum mismatch
        downloaded: h1:FP6YXyar5+eeXU8lBWDHJMZLHinornnILMJYBLgqKXk=
        go.sum:     h1:2qsuRm+bzgwSIKikigPASa2GhW8H2Dn4Qq7UxD8K/48=

SECURITY ERROR
This download does NOT match an earlier download recorded in go.sum.
The bits may have been replaced on the origin server, or an attacker may
have intercepted the download attempt.

Anyone have any ideas what I may be missing?

sm43 · 2021-09-09T06:15:10Z

@MarcusNoble could you rebase and then try again? :)

MarcusNoble · 2021-09-09T07:11:44Z

@sm43 Same problem I'm afraid.

tekton-robot · 2021-09-09T07:15:14Z

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/common/releases.go	63.6%	61.8%	-1.9
pkg/reconciler/kubernetes/tektonconfig/extension/dashboard.go	58.6%	55.7%	-2.9

Adds an optional boolean property to the TektonDashboard and the TektonConfig CRDs to control which version of the Dashboard manifest is used. If readonly is set to true the manifests with only the read RBAC permissions will be used. Signed-off-by: Vincent Demeester <[email protected]>

vdemeester · 2021-09-09T09:10:50Z

@MarcusNoble hum, interesting, I didn't run into this when doing it. I've amended your commit with the fixed codegen/dep updates.

vdemeester

/lgtm

tekton-robot · 2021-09-09T09:11:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nikhil-thomas, vdemeester

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~~ [nikhil-thomas,vdemeester]

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

tekton-robot · 2021-09-09T09:13:51Z

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/common/releases.go	63.6%	61.8%	-1.9
pkg/reconciler/kubernetes/tektonconfig/extension/dashboard.go	58.6%	55.7%	-2.9

MarcusNoble · 2021-09-09T09:36:26Z

Thanks @vdemeester. I'm guessing it's something I've got messed up in my env.

tekton-robot requested review from piyush-garg and sm43 September 6, 2021 07:20

tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 6, 2021

tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 6, 2021

nikhil-thomas reviewed Sep 6, 2021

View reviewed changes

tekton-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 8, 2021

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 8, 2021

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 9, 2021

vdemeester approved these changes Sep 9, 2021

View reviewed changes

tekton-robot assigned vdemeester Sep 9, 2021

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 9, 2021

tekton-robot merged commit 75c06f1 into tektoncd:main Sep 9, 2021

AverageMarcus mentioned this pull request Oct 23, 2021

Fix build - missing dashboard files #464

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add readonly property to Tekton Dashboard component #402

Add readonly property to Tekton Dashboard component #402

MarcusNoble commented Sep 6, 2021 •

edited

Loading

tekton-robot commented Sep 6, 2021

nikhil-thomas commented Sep 6, 2021

nikhil-thomas Sep 6, 2021

nikhil-thomas commented Sep 6, 2021

nikhil-thomas Sep 6, 2021

nikhil-thomas Sep 6, 2021

MarcusNoble commented Sep 6, 2021

nikhil-thomas commented Sep 8, 2021

nikhil-thomas commented Sep 8, 2021

tekton-robot commented Sep 8, 2021

MarcusNoble commented Sep 9, 2021

sm43 commented Sep 9, 2021

MarcusNoble commented Sep 9, 2021

tekton-robot commented Sep 9, 2021

vdemeester commented Sep 9, 2021

vdemeester left a comment

tekton-robot commented Sep 9, 2021

tekton-robot commented Sep 9, 2021

MarcusNoble commented Sep 9, 2021

		if [[ ${releaseFileName} == "tekton-dashboard-release-readonly" ]]; then
		dir="dashboard-readonly"

Add readonly property to Tekton Dashboard component #402

Add readonly property to Tekton Dashboard component #402

Conversation

MarcusNoble commented Sep 6, 2021 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Sep 6, 2021

nikhil-thomas commented Sep 6, 2021

nikhil-thomas Sep 6, 2021

Choose a reason for hiding this comment

nikhil-thomas commented Sep 6, 2021

nikhil-thomas Sep 6, 2021

Choose a reason for hiding this comment

nikhil-thomas Sep 6, 2021

Choose a reason for hiding this comment

MarcusNoble commented Sep 6, 2021

nikhil-thomas commented Sep 8, 2021

nikhil-thomas commented Sep 8, 2021

tekton-robot commented Sep 8, 2021

MarcusNoble commented Sep 9, 2021

sm43 commented Sep 9, 2021

MarcusNoble commented Sep 9, 2021

tekton-robot commented Sep 9, 2021

vdemeester commented Sep 9, 2021

vdemeester left a comment

Choose a reason for hiding this comment

tekton-robot commented Sep 9, 2021

tekton-robot commented Sep 9, 2021

MarcusNoble commented Sep 9, 2021

MarcusNoble commented Sep 6, 2021 •

edited

Loading