Update cluster-role for cilium to prevent errors in agent startup #11466

foobaar · 2024-08-20T14:43:18Z

ciliumloadbalancerippools permissions exists in the cilium helm chart for version 1.13.0 https://github.com/cilium/cilium/blob/v1.13.0/install/kubernetes/cilium/templates/cilium-agent/clusterrole.yaml#L71

The agent also needs permissions to read/watch secrets for bgp auth secrets when using CiliumBGPPeeringPolicy with a secret.
Without the permissions for secrets, we see the following warns in the logs when cilium starts up on a node:

is forbidden: User \"system:serviceaccount:kube-system:cilium\" cannot list resource \"secrets\" in API group \"\" in the namespace \"kube-system\"" subsys=k8s
time="2024-08-20T14:18:16Z" level=warning msg="github.com/cilium/cilium/pkg/k8s/resource/resource.go:808: failed to list *v1.Secret: secrets is forbidden: User \"system:serviceaccount:kube-system:cilium\" cannot list resource \"secrets\" in API group \"\" in the namespace \"kube-system\"" subsys=klog
time="2024-08-20T14:18:16Z" level=error msg=k8sError error="github.com/cilium/cilium/pkg/k8s/resource/resource.go:808: Failed to watch *v1.Secret: failed to list *v1.Secret: secrets is forbidden: User \"system:serviceaccount:kube-system:cilium\" cannot list resource \"secrets\" in API group \"\" in the namespace \"kube-system\"" subsys=k8s

What type of PR is this?
/kind bug

What this PR does / why we need it:
Gives the cilium agent required permissions to watch loadbalancerippools and secrets

Does this PR introduce a user-facing change?:
NONE

Release Note:

Fix Cilium agent permission can't read loadbalancerippools and secrets

** Cilium Config

apiVersion: v1
data:
  agent-health-port: "9879"
  auto-direct-node-routes: "False"
  bgp-secrets-namespace: kube-system
  bpf-ct-global-any-max: "262144"
  bpf-ct-global-tcp-max: "524288"
  bpf-lb-acceleration: disabled
  bpf-lb-mode: dsr
  bpf-map-dynamic-size-ratio: "0.0025"
  cgroup-root: /run/cilium/cgroupv2
  clean-cilium-bpf-state: "false"
  clean-cilium-state: "false"
  cluster-name: default
  cluster-pool-ipv4-cidr: 10.Y.0.0/14
  cluster-pool-ipv4-mask-size: "25"
  cni-exclusive: "True"
  cni-log-file: /var/run/cilium/cilium-cni.log
  custom-cni-conf: "false"
  debug: "False"
  disable-cnp-status-updates: "True"
  enable-bgp-control-plane: "true"
  enable-bpf-clock-probe: "True"
  enable-bpf-masquerade: "False"
  enable-host-legacy-routing: "True"
  enable-hubble: "true"
  enable-ip-masq-agent: "False"
  enable-ipv4: "True"
  enable-ipv4-masquerade: "True"
  enable-ipv6: "False"
  enable-ipv6-masquerade: "True"
  enable-l2-announcements: "False"
  enable-metrics: "true"
  enable-remote-node-identity: "True"
  enable-well-known-identities: "False"
  etcd-config: |-
    ---
    endpoints:
      - https://10.X.0.36:2379
      - https://10.X.0.37:2379
      - https://10.X.0.38:2379
      - https://10.X.0.39:2379
      - https://10.X.0.40:2379

    # In case you want to use TLS in etcd, uncomment the 'ca-file' line
    # and create a kubernetes secret by following the tutorial in
    # https://cilium.link/etcd-config
    ca-file: "/etc/cilium/certs/ca_cert.crt"

    # In case you want client to server authentication, uncomment the following
    # lines and create a kubernetes secret by following the tutorial in
    # https://cilium.link/etcd-config
    key-file: "/etc/cilium/certs/key.pem"
    cert-file: "/etc/cilium/certs/cert.crt"
  hubble-disable-tls: "false"
  hubble-listen-address: :4244
  hubble-metrics: dns drop tcp flow icmp http
  hubble-metrics-server: :9965
  hubble-tls-cert-file: /var/lib/cilium/tls/hubble/server.crt
  hubble-tls-client-ca-files: /var/lib/cilium/tls/hubble/client-ca.crt
  hubble-tls-key-file: /var/lib/cilium/tls/hubble/server.key
  identity-allocation-mode: kvstore
  ipam: cluster-pool
  ipv4-native-routing-cidr: 10.0.0.0/8
  kube-proxy-replacement: strict
  kvstore: etcd
  kvstore-opt: '{"etcd.config": "/var/lib/etcd-config/etcd.config"}'
  monitor-aggregation: medium
  monitor-aggregation-flags: all
  operator-api-serve-addr: 127.0.0.1:9234
  operator-prometheus-serve-addr: :9963
  preallocate-bpf-maps: "False"
  prometheus-serve-addr: :9962
  routing-mode: native
  sidecar-istio-proxy-image: cilium/istio_proxy
  write-cni-conf-when-ready: /host/etc/cni/net.d/05-cilium.conflist

ciliumloadbalancerippools permissions exists in the cilium helm chart for version 1.13.0 https://github.com/cilium/cilium/blob/v1.13.0/install/kubernetes/cilium/templates/cilium-agent/clusterrole.yaml#L71 The agent also needs permissions to read/watch secrets for bgp auth secrets when using CiliumBGPPeeringPolicy with a secret.

k8s-ci-robot · 2024-08-20T14:43:28Z

Hi @foobaar. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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-sigs/prow repository.

tico88612 · 2024-08-21T01:38:25Z

@foobaar Please add release note block.

Fix Cilium agent permission can't read loadbalancerippools and secrets

/ok-to-test

tico88612 · 2024-08-21T04:24:04Z

Please fix release note block.

Like this:

kubespray/.github/PULL_REQUEST_TEMPLATE.md

Lines 42 to 44 in 2f84567

    
           ```release-note 
        
           ```

tico88612 · 2024-08-21T04:24:52Z

/retest

foobaar · 2024-08-21T13:21:34Z

Updated release note

tico88612 · 2024-08-21T14:32:14Z

/retest

tico88612

/lgtm

k8s-ci-robot · 2024-08-22T00:30:47Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: foobaar, tico88612
Once this PR has been reviewed and has the lgtm label, please assign cristicalin for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

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

tico88612 · 2024-08-22T12:48:46Z

/retest-failed

foobaar · 2024-09-05T01:31:32Z

Hey is there anything additional you want from me on this MR?

tico88612 · 2024-09-05T02:26:19Z

@yankay This PR is about cilium agent fix. Please take a look, thanks.

VannTen · 2024-09-05T05:01:30Z

roles/network_plugin/cilium/templates/cilium/cr.yml.j2

@@ -28,6 +28,7 @@ rules:
  - pods
  - endpoints
  - nodes
+  - secrets


AFAICT the secrets permission in the helm charts is only get for secrets, should this be as tight here ?

I see the following errors in Cilium when i dont give watch/list

When I only have get permissions for secrets, I get the following Warn:

time="2024-09-05T18:09:04Z" level=warning msg="github.com/cilium/cilium/pkg/k8s/resource/resource.go:808: failed to list *v1.Secret: secrets is forbidden: User \"system:serviceaccount:kube-system:cilium\" cannot list resource \"secrets\" in API group \"\" in the namespace \"kube-system\"" subsys=klog time="2024-09-05T18:09:04Z" level=error msg=k8sError error="github.com/cilium/cilium/pkg/k8s/resource/resource.go:808: Failed to watch *v1.Secret: failed to list *v1.Secret: secrets is forbidden: User \"system:serviceaccount:kube-system:cilium\" cannot list resource \"secrets\" in API group \"\" in the namespace \"kube-system\"" subsys=k8s

When I only have get,list permissions for secrets, I get the following Warn:

time="2024-09-05T18:11:59Z" level=error msg=k8sError error="github.com/cilium/cilium/pkg/k8s/resource/resource.go:808: Failed to watch *v1.Secret: unknown (get secrets)" subsys=k8s

Hi @foobaar, Did you see the errors in cilium-agent pod? Look at https://github.com/cilium/cilium/blob/c9723a8df3cfa336da1f8457a864105d8349acfe/install/kubernetes/cilium/templates/cilium-agent/clusterrole.yaml#L66, I don't see that these extra list/watch permissions are needed cilium upstream, maybe we need to investigate further. which cilium version you used?

I did/do see these errors in the cilium-agent pod.
The log messages I posted are from the cilium agent pod without list/watch permissions.
The version of cilium I am using is the current default for kubespray which is: 1.15.4

I can remove the list/watch secrets from the MR if this is delaying/blocking this MR.
Get secrets and ciliumloadbalancerippools are required by the agent though.

Updated the PR to only get secrets and ciliumloadbalancerippools.
These two are missing in cluster role for cilium in kubespray

As for the list/watch, i will report that to cilium in a separate issue.

Updated the PR to only get secrets and ciliumloadbalancerippools.
These two are missing in cluster role for cilium in kubespray

Yes, It's better to report that to cilium in a separate issue, we should keep consites with the upstream.

Sounds good! I will so that. Thanks @cyclinder

I have updated the MR

yankay · 2024-09-12T03:10:43Z

Hi @cyclinder

would you please help to review it

k8s-ci-robot · 2024-09-13T12:27:36Z

New changes are detected. LGTM label has been removed.

tico88612 · 2024-09-16T14:46:52Z

/retest-failed

tico88612 · 2024-09-18T11:50:33Z

/retest-failed

foobaar · 2024-09-18T14:51:31Z

Hey! Is there anything else you want from me for this PR?

tico88612 · 2024-09-19T00:30:23Z

/retest

k8s-ci-robot requested review from MrFreezeex and tico88612 August 20, 2024 14:43

k8s-ci-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 Aug 21, 2024

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Aug 21, 2024

tico88612 approved these changes Aug 22, 2024

View reviewed changes

k8s-ci-robot assigned tico88612 Aug 22, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 22, 2024

VannTen reviewed Sep 5, 2024

View reviewed changes

Remove list/watch permissions for secrets

f7374c4

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 13, 2024

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 13, 2024

Remove secrets from list/watch permissions

2b94a4f

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 13, 2024

VannTen mentioned this pull request Sep 13, 2024

[CI] Redundant pipelines on push #11532

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update cluster-role for cilium to prevent errors in agent startup #11466

Update cluster-role for cilium to prevent errors in agent startup #11466

foobaar commented Aug 20, 2024 •

edited

Loading

k8s-ci-robot commented Aug 20, 2024

tico88612 commented Aug 21, 2024

tico88612 commented Aug 21, 2024

tico88612 commented Aug 21, 2024

foobaar commented Aug 21, 2024

tico88612 commented Aug 21, 2024

tico88612 left a comment

k8s-ci-robot commented Aug 22, 2024

tico88612 commented Aug 22, 2024

foobaar commented Sep 5, 2024

tico88612 commented Sep 5, 2024

VannTen Sep 5, 2024

foobaar Sep 5, 2024 •

edited

Loading

cyclinder Sep 12, 2024 •

edited

Loading

foobaar Sep 12, 2024

foobaar Sep 12, 2024

foobaar Sep 13, 2024

foobaar Sep 13, 2024

cyclinder Sep 14, 2024

foobaar Sep 14, 2024

foobaar Sep 16, 2024

yankay commented Sep 12, 2024

k8s-ci-robot commented Sep 13, 2024

tico88612 commented Sep 16, 2024

tico88612 commented Sep 18, 2024

foobaar commented Sep 18, 2024

tico88612 commented Sep 19, 2024

Update cluster-role for cilium to prevent errors in agent startup #11466

Are you sure you want to change the base?

Update cluster-role for cilium to prevent errors in agent startup #11466

Conversation

foobaar commented Aug 20, 2024 • edited Loading

k8s-ci-robot commented Aug 20, 2024

tico88612 commented Aug 21, 2024

tico88612 commented Aug 21, 2024

tico88612 commented Aug 21, 2024

foobaar commented Aug 21, 2024

tico88612 commented Aug 21, 2024

tico88612 left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Aug 22, 2024

tico88612 commented Aug 22, 2024

foobaar commented Sep 5, 2024

tico88612 commented Sep 5, 2024

Choose a reason for hiding this comment

foobaar Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

cyclinder Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yankay commented Sep 12, 2024

k8s-ci-robot commented Sep 13, 2024

tico88612 commented Sep 16, 2024

tico88612 commented Sep 18, 2024

foobaar commented Sep 18, 2024

tico88612 commented Sep 19, 2024

foobaar commented Aug 20, 2024 •

edited

Loading

foobaar Sep 5, 2024 •

edited

Loading

cyclinder Sep 12, 2024 •

edited

Loading