Skip to content

Helm keystore feature #10

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

karlnewell
Copy link

Adds optional keystore to helm chart.
This enables the use of a keystore provisioned externally or by cert-manager.
I moved the file locations so it was easier to mount Secrets without overwriting data in parent directories.
The certificate manifest is not included here but could be. I'm using the generated cert for multiple purposes so it doesn't necessarily make sense to put it in this repo (but I could add it as optional). The cert is used for both PolyNSI and SuPA Ingress as well as the PolyNSI keystore.
I also added a similar feature for a truststore is needed.

Adds feature to keep manual changes to configmap.
If enabled, the configmap resource is not overwritten during a helm upgrade and manual changes are preserved.

@karlnewell
Copy link
Author

My current override-polynsi-values.yaml file for reference:

# This is a YAML-formatted file.
# Declare variables to be passed into your templates.

image:
  repository: ghcr.io/workfloworchestrator/polynsi
  tag: 0.3.2
  pullPolicy: IfNotPresent
imagePullSecrets: []
nameOverride: ""
fullnameOverride: ""

service:
  type: ClusterIP
  port: 80  # polynsi:80 mapped to document_server_port (default 8080)
  grpc: 80  # polynsi-grpc:80 mapped to grpc_server_insecure_port (default 50051)

ingress:
  enabled: true
  className: public-web
  annotations:
    # nginx.ingress.kubernetes.io/auth-tls-pass-certificate-to-upstream: "true"
    nginx.ingress.kubernetes.io/auth-tls-verify-client: optional_no_ca
  hosts:
    - host: nsi.testing.ns.internet2.edu
      paths:
        - path: /soap
          pathType: Prefix
  tls:
   - secretName: polynsi-keystore
     hosts:
       - nsi.testing.ns.internet2.edu

resources:
   limits:
     cpu: 4
     memory: 2Gi
   requests:
     cpu: 400m
     memory: 2Gi

keystore:
  enabled: true
  secretName: polynsi-keystore

config:
  keep: true
  inline: |-
    application.properties: |-
      debug=true
      cxf.path=/soap
      soap.server.connection_provider.path=/connection/provider
      soap.server.connection_requester.path=/connection/requester
      grpc.server.port=9090
      grpc.client.connection_provider.address=static://supa-grpc.nsi.svc.cluster.local:80
      grpc.client.connection_provider.negotiationType=PLAINTEXT
      nl.surf.polynsi.verify-ssl-client-subject-dn=true
      nl.surf.polynsi.ssl-client-subject-dn-header=ssl-client-subject-dn
      server.ssl.enabled=false
      server.ssl.key-store=/usr/local/etc/polynsi/certs/polynsi-keystore.jks
      server.ssl.key-store-type=jks
      server.ssl.key-store-password=secret

- "-Djavax.net.ssl.trustStore=/usr/local/etc/polynsi/polynsi-truststore.jks"
- "-Djavax.net.ssl.keyStore=/usr/local/etc/polynsi/polynsi-keystore.jks"
- "-Djavax.net.ssl.trustStore=/usr/local/etc/polynsi/trust/polynsi-truststore.jks"
- "-Djavax.net.ssl.keyStore=/usr/local/etc/polynsi/certs/polynsi-keystore.jks"
Copy link
Member

Choose a reason for hiding this comment

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

This breaks existing uses of the this chart that supply the key and truststore as part of the config configmap that is mounted directly under /usr/local/etc/polynsi/. Is there a specific reason to put the stores in subdirectories?

Copy link
Author

Choose a reason for hiding this comment

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

There are challenges mounting multiple files from different secrets into the same directory. Let me revisit and see if I can get something to work.

I was thinking another option would be to dynamically set the path but that can't be done in a values.yaml file.

Copy link
Author

Choose a reason for hiding this comment

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

Updated to use projected volumes to mount multiple sources (configmaps and secrets) into /usr/local/etc/polynsi/

config:
# set to true to prevent helm from overwriting manual changes to configmap
keep: false
Copy link
Member

Choose a reason for hiding this comment

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

According to the Helm documentation, this annotation has a different effect:
The annotation helm.sh/resource-policy: keep instructs Helm to skip deleting this resource when a helm operation (such as helm uninstall, helm upgrade or helm rollback) would result in its deletion. However, this resource becomes orphaned.

Copy link
Author

Choose a reason for hiding this comment

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

You're right, that doesn't work as intended. I'll revert that change.

We would like to manage the configmap outside helm after initial install. This will become more important as we update the nl.surf.polynsi.client.certificate.distinguished-names field.

We could do something like

{{- if not (lookup "v1" "ConfigMap" .Release.Namespace "my-configmap") }}
apiVersion: v1
kind: ConfigMap
metadata:
  name: my-configmap
data:
  key: value
{{- end }}

that will only create the configmap if it doesn't exist. Maybe there's a way to make that if conditional dynamic - i.e., some orgs can use helm to maintain the configmap while others use helm to create it only on install.

How about this?
values.yaml

configmap:
  checkExistence: true  # Set to false to always create/update
  name: "my-configmap"

configmap.yaml

{{- $shouldCreate := true }}
{{- if .Values.configmap.checkExistence }}
{{- $shouldCreate = not (lookup "v1" "ConfigMap" .Release.Namespace .Values.configmap.name) }}
{{- end }}

{{- if $shouldCreate }}
apiVersion: v1
kind: ConfigMap
metadata:
  name: {{ .Values.configmap.name }}
data:
  key: value
{{- end }}

Copy link
Author

Choose a reason for hiding this comment

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

I modified the configmap with a checkExistence key. If true, the helm upgrade pulls the existing values from k8s and sets the configmap to those values. If false, the helm upgrade overwrites the configmap with values from the helm chart.

@karlnewell karlnewell force-pushed the feature/helm-keystore branch from 213a2b3 to 714a66a Compare July 2, 2025 15:34
This enables the use of a keystore provisioned externally or by
cert-manager.
Uses projected volumes
@karlnewell karlnewell force-pushed the feature/helm-keystore branch 2 times, most recently from 1a82799 to 2fea666 Compare July 2, 2025 19:27
@karlnewell karlnewell force-pushed the feature/helm-keystore branch from b287aaf to 77e1a5d Compare July 2, 2025 19:48
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