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

NVSHAS-7518 auto rotate internal certs #400

Closed

Conversation

holyspectral
Copy link
Contributor

This PR is to support 5.4's internal certificate rotation.

  1. Update helm charts to support auto internal cert rotation.
  2. Via unit-test, make sure that when the feature is off, resources created is the same as before.

Note that this feature will only be enabled on workloads when .Values.image.tag > 5.3.10-0.

@andsont
Copy link
Contributor

andsont commented Jul 8, 2024

  1. add neuvector-binding-job-creation and neuvector-binding-cert-upgrader roles in the role.yaml
  2. rollback the role-least.yaml
  3. rolebinding.yaml
    5-1. add neuvector-binding-job-creation and neuvector-binding-cert-upgrader rolebindings
    5.2. add the followings for neuvector-binding-lease rolebinding
{{- if $oc3 }}
userNames:
- system:serviceaccount:{{ .Release.Namespace }}:{{ .Values.serviceAccount }}
{{- end }}
  1. use one condition of {{- if .Values.internal.autoGenerateCert }} for all required rolebindings in the rolebinding-least.yaml
  2. change -role- role to -binding- role for all yaml files. make a consistent naming convention
  3. the followings are missing in the .controller.certupgrader of the values.yaml
priorityClassName:
podLabels: {}
podAnnotations: {}
nodeSelector:
  {}
  # key1: value1
  # key2: value2
runAsUser: # MUST be set for Rancher hardened cluster
9. the namespace and labels are missing in the controller-secret.yaml, controller-lease.yaml and upgrader-lease.yaml
  1. suggest to remove the following in values.yaml
  certupgrader:
    imagePullPolicy: IfNotPresent

hence, remove the following in controller-deployment.yaml

      initContainers:
        - name: init
          imagePullPolicy: {{ .Values.controller.certupgrader.imagePullPolicy }}
      containers:
        - name: neuvector-controller-pod
          imagePullPolicy: Always
  1. always enable list,watch verbs in role and enable enforcer,scanner and registry-adapter service accounts in rolebinding. remove the if condition.
{{- if .Values.internal.autoGenerateCert }}
  - list
  - watch
{{- end }}
{{- if .Values.internal.autoGenerateCert }}
- kind: ServiceAccount
  name: enforcer
  namespace: {{ .Release.Namespace }}
- kind: ServiceAccount
  name: scanner
  namespace: {{ .Release.Namespace }}
- kind: ServiceAccount
  name: registry-adapter
  namespace: {{ .Release.Namespace }}
{{- end }}

@holyspectral holyspectral force-pushed the auto-internal-certs-3 branch 2 times, most recently from ad1b3b4 to d53973f Compare July 10, 2024 17:56
@holyspectral holyspectral force-pushed the auto-internal-certs-3 branch from d53973f to ca74290 Compare July 17, 2024 22:16
@williamlin-suse
Copy link

upgrader-related rbac checking is missing in controller.
See VerifyNvK8sRBAC() in controller\resource\kubernetes_rbac.go (or search "neuvector-updater-pod" for reference)
(if it's enabled)

@holyspectral holyspectral force-pushed the auto-internal-certs-3 branch 2 times, most recently from deb64e3 to c4395f8 Compare August 7, 2024 16:34
@holyspectral holyspectral force-pushed the auto-internal-certs-3 branch 3 times, most recently from 86f1218 to 167e053 Compare August 15, 2024 21:15
1. New role and rolebinding for job creation and cert upgrader.
2. A new cronjob to run internal cert rotation regularly.
3. New options to enable cert generation and cert rotation.
@holyspectral holyspectral force-pushed the auto-internal-certs-3 branch from ded26a0 to 194e102 Compare August 21, 2024 17:29
@holyspectral holyspectral deleted the auto-internal-certs-3 branch November 18, 2024 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants