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

helm: add GCP CCM permissions for internal LBs #2474

Merged
merged 2 commits into from
Oct 19, 2023

Conversation

3u13r
Copy link
Member

@3u13r 3u13r commented Oct 18, 2023

Context

The GCP CCM hard-codes an outdated ClusterRole. We need to give it the required permissions again. See: kubernetes/cloud-provider-gcp#611

Proposed change(s)

  • Add a ClusterRoleBinding linking the used service account to the role we use for the CCMs on other CSPs.

Additional info

  • Any additional information or context

Checklist

  • Add labels (e.g., for changelog category)
  • Is PR title adequate for changelog?
  • Link to Milestone

@3u13r 3u13r added the bug fix Fixing a bug label Oct 18, 2023
@3u13r 3u13r added this to the v2.13.0 milestone Oct 18, 2023
@3u13r 3u13r requested a review from derpsteb as a code owner October 18, 2023 15:07
@netlify
Copy link

netlify bot commented Oct 18, 2023

Deploy Preview for constellation-docs canceled.

Name Link
🔨 Latest commit 2c8ae48
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/652ff6a000b964000910f190

@github-actions
Copy link
Contributor

Coverage report

Package Old New Trend
cli/internal/helm 49.50% 49.50% 🚧

name: cluster-admin
subjects:
- kind: ServiceAccount
name: cloud-provider
Copy link
Member

Choose a reason for hiding this comment

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

Can you provide an e2e test for this?
From what I can tell, there is no ServiceAccount called cloud-provider in our set up

Copy link
Member Author

Choose a reason for hiding this comment

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

E2E, gcp, 1.27, lb, 1:1 https://github.com/edgelesssys/constellation/actions/runs/6571600139
Or do you mean that I should add an E2E test which explicitly tests this feature?

Copy link
Member

Choose a reason for hiding this comment

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

No, just running the e2e test is fine.
But I'm still wondering who is supposed to consume this role binding as the referenced ServiceAccount does not exist (at least in our Helm charts)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's a default service account which simply exists in K8s and some (default) permissions have been removed from it. If it does not exists anymore, we'll have to create it, but currently it seems to work.

@3u13r 3u13r merged commit 498b5d6 into main Oct 19, 2023
9 checks passed
@3u13r 3u13r deleted the fix/helm/add-gcp-ccm-permissions branch October 19, 2023 08:58
@msanft msanft changed the title helm: add gcp ccm permissions for internal LBs helm: add GCP CCM permissions for internal LBs Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants