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

add helm config option to mount ca certs to cost model container (#3760) #3762

Closed
wants to merge 1 commit into from

Conversation

mittal-ishaan
Copy link
Contributor

  • add helm config option to mount ca certs to cost model container

  • update it to be configmap

  • shift from config map tp secret

  • nit fix

What does this PR change?

Does this PR rely on any other PRs?

How does this PR impact users? (This is the kind of thing that goes in release notes!)

Links to Issues or tickets this PR addresses or fixes

What risks are associated with merging this PR? What is required to fully test this PR?

How was this PR tested?

Have you made an update to documentation? If so, please provide the corresponding PR.

* add helm config option to mount ca certs to cost model container

* update it to be configmap

* shift from config map tp secret

* nit fix
@chipzoller
Copy link
Collaborator

Why a secret?

@mittal-ishaan
Copy link
Contributor Author

We wanted to give user an option to mount ca certs inside the constiner. We could have used configMap or secret. went ahead with secret for this one.

@chipzoller
Copy link
Collaborator

I recommend using a ConfigMap instead as it's the industry standard API resource for certificates.

@mittal-ishaan
Copy link
Contributor Author

thank you. let me correct it then.

@chipzoller
Copy link
Collaborator

And have you actually verified this will work in our container? I'm skeptical it will because of the need to run a command when adding anchors. Adding to the trust store directly usually does not require this but overwrites the store.

@jessegoodier
Copy link
Collaborator

And have you actually verified this will work in our container? I'm skeptical it will because of the need to run a command when adding anchors. Adding to the trust store directly usually does not require this but overwrites the store.

Thinking an initContainer?
We have a customer using this now. But let's continue to improve this and then document how to use it correctly.

@jessegoodier
Copy link
Collaborator

okay- proposing we move to something like this?

https://jfrog.com/help/r/artifactory-how-to-add-custom-ssl-certificates-to-an-artifactory-pod-using-helm-charts/how-to-update-the-ca-certificates-within-the-artifactory-pod

artifactory:
  customInitContainers: |
    - name: "sslsetup"
      image: "{{ .Values.initContainerImage }}"
      imagePullPolicy: "{{ .Values.artifactory.image.pullPolicy }}"
      securityContext:
        privileged: true
        runAsUser: 0
        runAsGroup: 0
        runAsNonRoot: false
      command:
        - 'sh'
        - '-c'
        - >
          mkdir -p /etc/pki/ca-trust/extracted/{edk2,java,openssl,pem};
          /usr/bin/update-ca-trust extract;
      volumeMounts:
        - mountPath: "{{ .Values.artifactory.persistence.mountPath }}"
          name: artifactory-volume
        - name: my-certs
          mountPath: "/etc/pki/ca-trust/source/anchors/mycustom.crt"
          subPath: mycustom.crt
        - name: ssl-path
          mountPath: "/etc/pki/ca-trust/extracted"
          readOnly: false
  customVolumes: |
   - name: ssl-path
     emptyDir: {}
   - name: my-certs
     configMap:
       name: my-certs
  customVolumeMounts: |
   - name: my-certs
     mountPath: /etc/pki/ca-trust/source/anchors/mycustom.crt
     subPath: mycustom.crt
   - name: ssl-path
     mountPath: /etc/pki/ca-trust/extracted

@mittal-ishaan
Copy link
Contributor Author

yes, this looks better to me. Let me get this done.

@chipzoller
Copy link
Collaborator

Yes, that's what I am driving at. How does this work unless someone/something executes a command after the new anchors are provided to merge them into the store? Normally on Red Hat you have to run update-ca-trust I believe in order to make that happen.

@mittal-ishaan
Copy link
Contributor Author

Yes, we earlier we were expecting the user to run update-ca-trust inside the container. but having it done in a init container is better.

@chipzoller
Copy link
Collaborator

Requiring users run commands inside of a container should always be avoided at all costs. If the initContainer works, that'd be a better approach.

@chipzoller
Copy link
Collaborator

chipzoller commented Dec 16, 2024

Superseded by #3763

@chipzoller chipzoller closed this Dec 16, 2024
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