Skip to content

Conversation

tobsyg
Copy link

@tobsyg tobsyg commented Oct 6, 2025

  • Tests written and linted ℹ︎
  • Documentation written ℹ︎
  • Commit history is tidy ℹ︎

What this does

Related to support case #

Rendering CA certs as optional

Notes for the reviewer

Attempt to run Snyk Kubernetes chart without specifying the extraCaCerts and certsConfigMap via values
The secret should then not be mounted into the Snyk pod prior to running 00115307

@tobsyg tobsyg requested a review from a team as a code owner October 6, 2025 09:38
@tobsyg tobsyg requested a review from bdemeo12 October 6, 2025 09:38
@CLAassistant
Copy link

CLAassistant commented Oct 6, 2025

CLA assistant check
All committers have signed the CLA.

@snyk-pr-review-bot
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Clarification Needed

The PR description mentions "The secret should then not be mounted", but the code change at lines 287-292 makes a configMap volume optional. Please confirm if this is the intended resource type or if a secret is also involved and was missed.

{{- if .Values.certsConfigMap }}
- name: ssl-certs
  configMap:
    name: {{ .Values.certsConfigMap }}
    optional: true
{{- end }}

@tobsyg
Copy link
Author

tobsyg commented Oct 9, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Clarification Needed
The PR description mentions "The secret should then not be mounted", but the code change at lines 287-292 makes a configMap volume optional. Please confirm if this is the intended resource type or if a secret is also involved and was missed.

{{- if .Values.certsConfigMap }}
- name: ssl-certs
  configMap:
    name: {{ .Values.certsConfigMap }}
    optional: true
{{- end }}

I confirm this is a typo in the description, the actual resource type is a configMap

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.

2 participants