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

Extending pipeline promotions token security documentation #3540

Merged
merged 19 commits into from
Aug 3, 2023

Conversation

enekofb
Copy link
Contributor

@enekofb enekofb commented Mar 20, 2023

Closes weaveworks/weave-gitops-enterprise#2564

What changed?

Extending existing guidance to include the ability to create deny semantics required for least privilege on access to the credentials token

Why was this change made?
Cause otherwise, pipeline controller would have access to the secret but nothing would have avoid that other roles has access to.

How was this change implemented?
Docs

How did you validate the change?

Tested locally

Screenshot 2023-07-11 at 09 54 14

Release notes

Documentation Changes

the creation of roles that would allow access to the promotion credentials.

```yaml
apiVersion: pac.weave.works/v2beta2
Copy link
Contributor Author

@enekofb enekofb Mar 28, 2023

Choose a reason for hiding this comment

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

Screenshot 2023-03-28 at 09 27 41

- name: verb
type: string
required: true
value: "get" # do the same for list and watch
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do a couple of things here?

  1. Put the full version of this for users so that they can copy and paste.
  2. Check what happens when someone uses * for verbs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given policy library is enterprise feature, I cannot add it here.

@enekofb
Copy link
Contributor Author

enekofb commented Jun 29, 2023

@enekofb @squaremo

Assumptions / Actions:

Service Accounts

  • none but pipeline-controller pod can user / impersonate pipeline-controller SA

Extend RBAC

  • clusterrolebindings not allowed only admins
  • rolebindings not allowed for non-pipeline namespace

Extend Policy

  • restrict wildcards to secrets within my-pipeline-namespace
  • restrict list to secrets within my-pipeline-namespace
  • restrict as namespace owner I cannot create rolebinding nor clusterrolebindings to allow access anyone outside my ns
  • only allow get secret to secret by name to pipeline-controller SA

@enekofb enekofb force-pushed the promotion-apps-access-review branch from c203289 to 5553f3f Compare June 30, 2023 06:37
@enekofb enekofb requested review from squaremo and removed request for ahsayde June 30, 2023 09:58
@enekofb enekofb force-pushed the promotion-apps-access-review branch 2 times, most recently from 784b0f1 to e0b9341 Compare July 3, 2023 10:35
Copy link
Contributor Author

@enekofb enekofb left a comment

Choose a reason for hiding this comment

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

comments added from session with Michael and Kevin

@enekofb enekofb force-pushed the promotion-apps-access-review branch from c7891ce to 921e2e9 Compare July 10, 2023 10:37
@enekofb enekofb marked this pull request as ready for review July 10, 2023 10:38
@enekofb enekofb force-pushed the promotion-apps-access-review branch from 921e2e9 to af92de6 Compare July 10, 2023 10:39
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- policies/RBACProhibitWildcards/no-wildcard-on-resources-policy.yaml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MostafaMegahid how should we contribute these policies to the policy library ?

Copy link
Contributor Author

@enekofb enekofb Jul 19, 2023

Choose a reason for hiding this comment

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

Today we synced on this topic. the suggested path would be to

  1. add these policies to policy library. An example of this could https://github.com/weaveworks/policy-library/tree/main/policies/RBACClusterRoleClusterAdmin
  2. create a rbac best practices kustomization including the previous policies
  3. reference this kustomization in the documentation

A user would only need to use this kustomization
cc @squaremo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Draft PR for good practices within policy-library weaveworks/policy-library#31

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To review today with Mostafa

@enekofb enekofb force-pushed the promotion-apps-access-review branch from dddf388 to d7f6f6c Compare July 11, 2023 08:11
Copy link
Contributor

@squaremo squaremo left a comment

Choose a reason for hiding this comment

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

Just edits as suggestions, this time. Once we have nailed down how to use the policy library, I think we're good to go.

website/docs/pipelines/promoting-applications.mdx Outdated Show resolved Hide resolved
website/docs/pipelines/promoting-applications.mdx Outdated Show resolved Hide resolved
website/docs/pipelines/promoting-applications.mdx Outdated Show resolved Hide resolved
website/docs/pipelines/promoting-applications.mdx Outdated Show resolved Hide resolved
website/docs/pipelines/promoting-applications.mdx Outdated Show resolved Hide resolved
website/docs/pipelines/promoting-applications.mdx Outdated Show resolved Hide resolved
website/docs/pipelines/promoting-applications.mdx Outdated Show resolved Hide resolved
website/docs/pipelines/promoting-applications.mdx Outdated Show resolved Hide resolved
website/docs/pipelines/promoting-applications.mdx Outdated Show resolved Hide resolved
@enekofb enekofb force-pushed the promotion-apps-access-review branch from dac27b9 to 8e86041 Compare July 25, 2023 07:41
@enekofb enekofb requested a review from squaremo July 31, 2023 14:42
Copy link
Contributor

@squaremo squaremo left a comment

Choose a reason for hiding this comment

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

Thorough work Eneko, thank you so much for following this through. What a journey :-)

enekofb and others added 18 commits August 3, 2023 15:38
This is Eneko and Michael going through together and refining some of the points.
Co-authored-by: Michael Bridgen <[email protected]>
Co-authored-by: Michael Bridgen <[email protected]>
@enekofb enekofb force-pushed the promotion-apps-access-review branch from 31d6e68 to da28ee2 Compare August 3, 2023 14:38
@enekofb enekofb merged commit 4953c55 into main Aug 3, 2023
15 checks passed
@enekofb enekofb deleted the promotion-apps-access-review branch August 3, 2023 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants