overlay and readme changes custom CA and use env as verify in Elyra #669

shalberd · 2022-08-31T19:52:02Z

Description

Background OCP / Openshift central

https://docs.openshift.com/container-platform/4.8/networking/configuring-a-custom-pki.html#installation-configure-proxy_configuring-a-custom-pki

key concept: additionalTrustBundle PEM-Format files

Configmap with label config.openshift.io/inject-trusted-cabundle: "true" provides a block of PEM Format CA files with key
ca-bundle.crt containing the user-provided (additionalTrustBundle e.g. from private PKI) and system CA certificates in a single bundle file.

opendatahub-io-contrib/jupyterhub-odh#137
elyra-ai/elyra#2912
elyra-ai/elyra#2797 (comment)

I tested this overlay output on a local macbook with kustomize v4.5.6 and command

kustomize build ./odh-manifests/jupyterhub/jupyterhub/overlays/trusted-ca-bundle-path

ConfigMap jupyterhub-cfg needs to have the following new section / value of key filled in which was previously empty

jupyterhub_config.py: |-
    if "TRUSTED_CA_BUNDLE_PATH" in os.environ:
          TRUSTED_CA_BUNDLE_PATH = os.environ.get("TRUSTED_CA_BUNDLE_PATH")
          basepath, filename = os.path.split({TRUSTED_CA_BUNDLE_PATH})
          spawner = c.OpenShiftSpawner`
     
    etc.

ConfigMap jupyter-singleuser-profiles needs to have the following new env filled in, as long as parameter trusted_ca_bundle_path was given in parameters section of KfDefs kustomizeConfig

profiles:
    - name: globals
      env:
        - name: TRUSTED_CA_BUNDLE_PATH
          value: /opt/app-root/etc/jupyter/custom/cacerts/trustedcas.pem

 - kustomizeConfig:
     overlays:
     - trusted-ca-bundle-path
     parameters:
       - name: trusted_ca_bundle_path
         value: "/opt/app-root/etc/jupyter/custom/cacerts/trustedcas.pem"
     repoRef:
       name: manifests
       path: jupyterhub/jupyterhub
   name: jupyterhub

How Has This Been Tested?

Merge criteria:

The commits are squashed in a cohesive manner and have meaningful messages.
Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
The developer has manually tested the changes and verified that the changes work

openshift-ci · 2022-08-31T19:52:14Z

Hi @shalberd. Thanks for your PR.

I'm waiting for a opendatahub-io 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/test-infra repository.

LaVLaS · 2022-08-31T20:36:34Z

jupyterhub/jupyterhub/base/jupyterhub-singleuser-profiles-configmap.yaml

+          - name: TRUSTED_CA_BUNDLE_PATH
+            value: $(trusted_ca_bundle_path)


This should be moved to the overlay and added via a kustomize patch so that the base deployment is not affected by the environment variable

I wondered about effects, too. But when I remove the parameter s3_endpoint_url as a paremter in KfDef kustomizeConfig https://opendatahub.io/docs/administration/advanced-installation/object-storage.html, that env var S3_ENDPOINT_URL, too, was not present in both jupyterhub and spawned containers anymore. So I figured this approach was ok. What I mean is, if by default the param is empty, then the env, too, is non-existent.

moved to the overlay, that is, jupyterhub-singleuser-profiles-configmap in its entirety.

shalberd · 2022-08-31T20:37:42Z

jupyterhub/jupyterhub/overlays/trusted-ca-bundle-path/jupyterhub-configmap.yaml

+  path: /data/jupyterhub_config.py
+  value: |-
+    if "TRUSTED_CA_BUNDLE_PATH" in os.environ:
+          TRUSTED_CA_BUNDLE_PATH = os.environ.get("TRUSTED_CA_BUNDLE_PATH")


I have tested this without env TRUSTED_CA_BUNDLE_PATH, but with hardcoded path in line 43. Result was the correctly mounted pem file from new configmap trusted-cabundle that in Openshift with central additional trust CA settings auto-injects a file section ca-bundle.crt that is referenced and used here. This worked very nicely, see discussion at opendatahub-io-contrib/jupyterhub-odh#137 (comment)

this new jupyterhub.config.py section was added in the output of "kustomize build" as wanted in my local macbook tests.

LaVLaS · 2022-08-31T20:42:45Z

@shalberd Thank you for submitting this PR. In order to add this, I want to make sure that the base JupyterHub manifests is not modified and the application of any changes in support of reading a custom CA for JupyterHub is contained in the trusted-ca-path overlay

shalberd · 2022-09-01T06:19:37Z

I have medium-level experience with overlays in Kustomize. Will try, though. Especially wondering about where to define the parameter and how to define the add syntax to the configmap that adds the env variable. I have before added envs in overlays to DeploymentConfigs, but not to this special type of configmap for jupyterhub-singleuser-profiles.

…rrect json merge section for jupyterhub singleuser and jupyterhub-cfg configmap

shalberd · 2022-09-01T07:48:53Z

...hub/jupyterhub/overlays/trusted-ca-bundle-path/jupyterhub-singleuser-profiles-configmap.yaml

@@ -0,0 +1,5 @@
+- op: add
+  path: /data/jupyterhub-singleuser-profiles.yaml/profiles[0]/env/-


here, I need some input on whether this path reference and operation is good with respect to the original file with an existing env in base.

shalberd · 2022-09-01T07:49:46Z

jupyterhub/jupyterhub/overlays/trusted-ca-bundle-path/kustomization.yaml

+bases:
+  - ../../base
+resources:
+  - trusted-cabundle-configmap.yaml


this is the new configmap containing the PEM CAs (roots, intermediates) all-in-one file, auto-injected by openshift with key ca-bundle.crt https://docs.openshift.com/container-platform/4.8/networking/configuring-a-custom-pki.html#certificate-injection-using-operators_configuring-a-custom-pki

This could potentially also be used as golang env SSL_CERT_FILE in the operator for manifests tar.gz download from enterprise-internal locations secured with SSL certificates that are based on non-publicly-trusted CAs, as this file contains all system-level RHCOS publicly-trusted-CAs in addition to additonally-specified enterprise private PKI CAs. In RedHat's words: "Once your custom CA certificate is added to the cluster via ConfigMap, the Cluster Network Operator merges the user-provided and system CA certificates into a single bundle and injects the merged bundle" That would solve the cert verify problem at the golang/operator level as well. https://go.dev/src/crypto/x509/root_unix.go

Back to the issue at hand: The configmap has a a bunch of PEM-Format certificates all in one file. Order is always RootCAs and then IntermediateCAs.

shalberd · 2022-09-01T07:51:11Z

jupyterhub/jupyterhub/overlays/trusted-ca-bundle-path/kustomization.yaml

+  - ../../base
+resources:
+  - trusted-cabundle-configmap.yaml
+patchesJson6902:


this notation should make sure the ops "replace" works for jupyterhub-configmap.yaml section data/jupyterhub_config.py in base. In the past, I only worked with patchesStrategicMerge yaml-based and ArgoCD in the past, but this approach seems cool, too as long as the json structure is well-defined.

Worked fine with kustomize build output for jupyterhub-configmap.yaml / jupyterhub-cfg ConfigMap.

shalberd · 2022-09-02T15:37:44Z

@shalberd Thank you for submitting this PR. In order to add this, I want to make sure that the base JupyterHub manifests is not modified and the application of any changes in support of reading a custom CA for JupyterHub is contained in the trusted-ca-path overlay

Yes. I just noticed there are changes ongoing to go with all KF Notebook Controller and Jupyter Notebooks instead of Jupyterhub. Since Elyra runs in the KF Notebook controller images soon, too, should we go ahead and bring in those custom PKI trusted CA changes from here to KF Notebook instead? Regarding location KF Notebook spawner, for adding a new volumeMount / volume filled with a file from a configmap containing a list of trusted CAs, is this the right location? https://github.com/opendatahub-io/odh-dashboard/tree/main/frontend/src/pages/notebookController/frontend/src/pages/notebookController/screens/server/SpawnerPage.tsx#L282 I would like to add an overlay. My aim is: a) add a new configmap that contains auto-injected system CAs and additonal trusted CAs in a file. b) have that file mapped to a volume and mount it to a location in the spawned container. c) Add an env variable TRUSTED_CA_BUNDLE_PATH for use by Elyra.

shalberd · 2022-09-13T07:10:15Z

@LaVLaS @vpavlin

Multiline scalar value pipe symbol in configmap

https://github.com/opendatahub-io/odh-manifests/blob/master/jupyterhub/jupyterhub/base/jupyterhub-singleuser-profiles-configmap.yaml#L6

seems to make add and replace operations with jsonpath kustomize / JsonPatches6902 for e.g. adding to env variables next to impossible. Everything after key /data/jupyterhub-singleuser-profiles.yaml is taken as literal ...

e.g.

versus without pipe symbol

I tested on my local macbook with kustomize v4.5.6. Getting output "add operation does not apply: doc is missing path"

--> found no other way than to include the entire configmap with the new env variable in the overlay instead of using merges via kustomize patchesStrategicMerge in overlay kustomization.yaml.

I cannot test this build result / yaml output result on the cluster, the kustomize aspects, that is, because I cannot download adapted recipes from an internal location that in our case has private PKI CAs. That, as I have mentioned, is a major drawback of the url definition / download functionality in kfdef manifests, but for another PR, I have ideas with regards to golang.

…ap to entirety due to inability to do JsonPatches6902 or strategic merge with scalar text value in configmap

shalberd · 2022-09-14T08:45:15Z

...hub/jupyterhub/overlays/trusted-ca-bundle-path/jupyterhub-singleuser-profiles-configmap.yaml

+        env:
+          - name: S3_ENDPOINT_URL
+            value: $(s3_endpoint_url)
+          - name: TRUSTED_CA_BUNDLE_PATH


this is the new ENV for this PR

as long as the parameter is given /filled-out, either in params.env or in KfDef kustomizeConfig, the resulting configmap has the new env plus value correctly in it. Tested locally with kustomize build odh-manifests/jupyterhub/jupyterhub/overlays/trusted-ca-bundle-path

shalberd · 2022-09-14T08:46:36Z

...hub/jupyterhub/overlays/trusted-ca-bundle-path/jupyterhub-singleuser-profiles-configmap.yaml

+        - 's2i-spark-minimal-notebook:py36-spark2.4.5-hadoop2.7.3'
+        env:
+          - name: PYSPARK_SUBMIT_ARGS
+            value: '--conf spark.cores.max=2 --conf spark.executor.instances=2 --conf spark.executor.memory=1G --conf spark.executor.cores=1 --conf spark.driver.memory=2G --packages com.amazonaws:aws-java-sdk:1.7.4 org.apache.hadoop:hadoop-aws:2.7.3 io.xskipper:xskipper-core_2.11:1.1.1 pyspark-shell'


I noticed that in --packages in the original file, where was a mix of commata and empty space separators for the --packages argument of PYSPARK_SUBMIT_ARGS. I used the correct notation here, with empty spaces between the packages listed.

In any case, there should not be a mix of comma-separations and empty spaces.

shalberd · 2022-09-14T09:41:31Z

I tested this as much as possible with kustomize locally, looking good from my perspective. Also added testing instructions at the very top of this PR.

Related PR for the operator and its download functionality (manifests tar.gz) from server locations secured with certs based on private PKI is here:

opendatahub-io/opendatahub-operator#173

…or-volume-mount-path

LaVLaS · 2022-09-20T04:09:00Z

@shalberd Since we don't have access to an environment to reproduce the original issue, all we can do is verify the default JH deployment with no overlay works as intended. I will try to follow-up to see if the trusted-ca-bundle-path overlay is parsed successfully but that is the limit of what we can do considering we are switching to notebook controller for managing the lifecycle of Jupyter notebooks

shalberd · 2022-09-20T05:13:00Z

"Since we don't have access to an environment to reproduce the original issue, all we can do is verify the default JH deployment with no overlay works as intended."

That is fine by me, I did environment testing as much as possible from my part and overlay parsing testing with Kustomize, so it is good.

LaVLaS

/lgtm

@shalberd Thx for this update. As discussed I can only confirm that it does not break existing functionality in the environments that we have access to. Since we have moved to notebook-controller for managing the notebook lifecycle, this will most likely be the last update to JupyterHub in odh-manifests before we officially move it to the odh-contrib-manifests

openshift-ci · 2022-09-29T20:06:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LaVLaS

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~jupyterhub/OWNERS~~ [LaVLaS]

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

shalberd · 2022-10-02T09:39:10Z

No idea why ci/prow/odh-manifests-e2e test is failing. All files are in the correct place, as well as permissions.

…or-volume-mount-path

shalberd · 2022-10-02T09:42:26Z

/test odh-manifests-e2e

openshift-ci · 2022-10-02T09:42:41Z

@shalberd: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test odh-manifests-e2e

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/test-infra repository.

anishasthana · 2022-10-02T18:14:29Z

/ok-to-test

LaVLaS · 2022-10-04T14:11:28Z

/lgtm

overlay and readme changes

5dcc34f

openshift-ci bot requested review from anishasthana and VaishnaviHire August 31, 2022 19:52

openshift-ci bot added the needs-ok-to-test label Aug 31, 2022

jupyterhub singleuser configmap add env TRUSTED_CA_BUNDLE_PATH

4da67f8

shalberd mentioned this pull request Aug 31, 2022

add trusted-ca configmap support for Kubernetes and Openshift 4.x opendatahub-io-contrib/jupyterhub-odh#137

Open

shalberd added 3 commits August 31, 2022 22:10

added files in overlay

5c925a4

delete build directory from wrong subfolder

56a4e99

removed mess in folder structure

7705b77

LaVLaS reviewed Aug 31, 2022

View reviewed changes

shalberd commented Aug 31, 2022

View reviewed changes

moved new env add from base singleuser configmap to overlay. added co…

45933f4

…rrect json merge section for jupyterhub singleuser and jupyterhub-cfg configmap

shalberd commented Sep 1, 2022

View reviewed changes

shalberd mentioned this pull request Sep 2, 2022

Cleanup the kfdef and use odh-core kfdef as the standard for ODH Core deployments #672

Merged

3 tasks

shalberd added 3 commits September 14, 2022 09:30

changed additonal env entry in jupyterhub-singleuser-profiles configm…

6dbf93e

…ap to entirety due to inability to do JsonPatches6902 or strategic merge with scalar text value in configmap

typo in overlay kustomization.yaml

280813f

added jupyterhub singleuser profiles configmap via strategic merge

95176cb

shalberd commented Sep 14, 2022

View reviewed changes

added patchesJson6902 for jupyterhub-configmap

04fcb10

Merge branch 'master' into add-trusted-ca-configmap-support-and-env-f…

a7b3726

…or-volume-mount-path

LaVLaS self-requested a review September 29, 2022 20:02

LaVLaS approved these changes Sep 29, 2022

View reviewed changes

openshift-ci bot assigned LaVLaS Sep 29, 2022

openshift-ci bot added the lgtm label Sep 29, 2022

openshift-ci bot added the approved label Sep 29, 2022

Merge branch 'master' into add-trusted-ca-configmap-support-and-env-f…

daa6058

…or-volume-mount-path

openshift-ci bot removed the lgtm label Oct 2, 2022

openshift-ci bot added ok-to-test and removed needs-ok-to-test labels Oct 2, 2022

openshift-ci bot added the lgtm label Oct 4, 2022

openshift-merge-robot merged commit 0edd65c into opendatahub-io:master Oct 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

overlay and readme changes custom CA and use env as verify in Elyra #669

overlay and readme changes custom CA and use env as verify in Elyra #669

shalberd commented Aug 31, 2022 •

edited

Loading

openshift-ci bot commented Aug 31, 2022

LaVLaS Aug 31, 2022

shalberd Aug 31, 2022 •

edited

Loading

shalberd Sep 14, 2022

shalberd Aug 31, 2022 •

edited

Loading

LaVLaS commented Aug 31, 2022

shalberd commented Sep 1, 2022

shalberd Sep 1, 2022 •

edited

Loading

shalberd Sep 1, 2022 •

edited

Loading

shalberd Sep 1, 2022 •

edited

Loading

shalberd commented Sep 2, 2022 •

edited

Loading

shalberd commented Sep 13, 2022 •

edited

Loading

shalberd Sep 14, 2022

shalberd Sep 14, 2022 •

edited

Loading

shalberd Sep 14, 2022 •

edited

Loading

shalberd commented Sep 14, 2022 •

edited

Loading

LaVLaS commented Sep 20, 2022

shalberd commented Sep 20, 2022

LaVLaS left a comment

openshift-ci bot commented Sep 29, 2022

shalberd commented Oct 2, 2022

shalberd commented Oct 2, 2022

openshift-ci bot commented Oct 2, 2022

anishasthana commented Oct 2, 2022

LaVLaS commented Oct 4, 2022

		- name: TRUSTED_CA_BUNDLE_PATH
		value: $(trusted_ca_bundle_path)

		@@ -0,0 +1,5 @@
		- op: add
		path: /data/jupyterhub-singleuser-profiles.yaml/profiles[0]/env/-

overlay and readme changes custom CA and use env as verify in Elyra #669

overlay and readme changes custom CA and use env as verify in Elyra #669

Conversation

shalberd commented Aug 31, 2022 • edited Loading

Description

How Has This Been Tested?

Merge criteria:

openshift-ci bot commented Aug 31, 2022

LaVLaS Aug 31, 2022

Choose a reason for hiding this comment

shalberd Aug 31, 2022 • edited Loading

Choose a reason for hiding this comment

shalberd Sep 14, 2022

Choose a reason for hiding this comment

shalberd Aug 31, 2022 • edited Loading

Choose a reason for hiding this comment

LaVLaS commented Aug 31, 2022

shalberd commented Sep 1, 2022

shalberd Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

shalberd Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

shalberd Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

shalberd commented Sep 2, 2022 • edited Loading

shalberd commented Sep 13, 2022 • edited Loading

shalberd Sep 14, 2022

Choose a reason for hiding this comment

shalberd Sep 14, 2022 • edited Loading

Choose a reason for hiding this comment

shalberd Sep 14, 2022 • edited Loading

Choose a reason for hiding this comment

shalberd commented Sep 14, 2022 • edited Loading

LaVLaS commented Sep 20, 2022

shalberd commented Sep 20, 2022

LaVLaS left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Sep 29, 2022

shalberd commented Oct 2, 2022

shalberd commented Oct 2, 2022

openshift-ci bot commented Oct 2, 2022

anishasthana commented Oct 2, 2022

LaVLaS commented Oct 4, 2022

shalberd commented Aug 31, 2022 •

edited

Loading

shalberd Aug 31, 2022 •

edited

Loading

shalberd Aug 31, 2022 •

edited

Loading

shalberd Sep 1, 2022 •

edited

Loading

shalberd Sep 1, 2022 •

edited

Loading

shalberd Sep 1, 2022 •

edited

Loading

shalberd commented Sep 2, 2022 •

edited

Loading

shalberd commented Sep 13, 2022 •

edited

Loading

shalberd Sep 14, 2022 •

edited

Loading

shalberd Sep 14, 2022 •

edited

Loading

shalberd commented Sep 14, 2022 •

edited

Loading