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 Custom CA Bundle Injection into DSP via DSPA #440

Merged
merged 1 commit into from
Nov 6, 2023

Conversation

HumairAK
Copy link
Contributor

@HumairAK HumairAK commented Oct 31, 2023

The issue resolved by this Pull Request:

Resolves #362

Description of your changes:

Add CA Injection capabilities to api server pod and pipeline s3 copy steps.

Testing instructions

Deploy DSPO

# git clone and checkout the PR branch
make deploy IMG=quay.io/opendatahub/data-science-pipelines-operator:pr-440

Deploy a Minio in a self-signed external OCP cluster (preferablly different from the dsp host cluster)

oc new-project minio
oc -n minio apply -f https://gist.githubusercontent.com/HumairAK/f1358278ab70dde2ee074d1a35f8f058/raw/f234ba638f408c3aff8ed836858d0d1e332f43be/minio.yaml

# Retrieve the route
$MINIO_ROUTE=$(oc get routes -n minio minio-secure --template={{.spec.host}})

On DSP host cluster, deploy a dspa, first without the cabundle:

dspa.yaml
kind: Secret
apiVersion: v1
metadata:
  name: s3secret
  namespace: dspa
stringData:
  customaccessKey: accesskey
  customsecretKey: secretkey
type: Opaque
---
apiVersion: datasciencepipelinesapplications.opendatahub.io/v1alpha1
kind: DataSciencePipelinesApplication
metadata:
  name: sample
  namespace: dspa
spec:
  apiServer:
    enableSamplePipeline: false
  objectStorage:
    externalStorage:
      bucket: mlpipeline
      host: $MINIO_ROUTE  # <------------- add the route retrieved earlier !
      s3CredentialsSecret:
        accessKey: customaccessKey
        secretKey: customsecretKey
        secretName: s3secret
      scheme: https
  mlpipelineUI:
    image: 'quay.io/opendatahub/odh-ml-pipelines-frontend-container:beta-ui'

Check DSPO logs, you should see an error mentioning x509 and instructions on adding a cabundle.
From the minio self-signed ocp cluster get the ca bundle from minio namespace:

oc -n minio get configmap kube-root-ca.crt -o yaml | yq '.data."ca.crt"'  > ca-bundle.crt

Put this value in a configmap:

kind: ConfigMap
apiVersion: v1
metadata:
  name: custom-minio-ca-bundle
data:
  ca-bundle.crt: |
    # contents of ca-bundle.crt

Deploy this in the DSPA namespace. Then update the dspa to be:

dspa.yaml
apiVersion: datasciencepipelinesapplications.opendatahub.io/v1alpha1
kind: DataSciencePipelinesApplication
metadata:
  name: sample
spec:
  apiServer:
    enableSamplePipeline: false
    cABundle:               # We add the configmap details here, mouse over this field in ocp to see the field descriptions
      configMapKey: ca-bundle.crt
      configMapName: custom-minio-ca-bundle
  database:
    disableHealthCheck: true
  objectStorage:
    externalStorage:
      bucket: mlpipeline
      host: $MINIO_ROUTE  # <------------- add the route retrieved earlier !
      s3CredentialsSecret:
        accessKey: customaccessKey
        secretKey: customsecretKey
        secretName: s3secret
      scheme: https
  mlpipelineUI:
    image: 'quay.io/opendatahub/odh-ml-pipelines-frontend-container:beta-ui'

Once deployed you will see the DSPA get deployed successfully, run an iris pipeline, make sure the artifact passing is successful (does s3 data get storec successfully? does pipeline run to completion) ?

Also confirm no regression, (if it's an http minio, does that still work?) etc.

Checklist

  • 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

@dsp-developers
Copy link
Contributor

A new image has been built to help with testing out this PR: quay.io/opendatahub/data-science-pipelines-operator:pr-440
An OCP cluster where you are logged in as cluster admin is required.

To use this image run the following:

cd $(mktemp -d)
git clone git@github.com:opendatahub-io/data-science-pipelines-operator.git
cd data-science-pipelines-operator/
git fetch origin pull/440/head
git checkout -b pullrequest 2c3f9d3515cef6c8548b05a1fb65b4c54c831baf
make deploy IMG="quay.io/opendatahub/data-science-pipelines-operator:pr-440"

More instructions here on how to deploy and test a Data Science Pipelines Application.

@dsp-developers
Copy link
Contributor

Change to PR detected. A new PR build was completed.
A new image has been built to help with testing out this PR: quay.io/opendatahub/data-science-pipelines-operator:pr-440

This change will allow users to specify a configmap that contains a ca
bundle. This ca bundle is injected into the api server pod.
@dsp-developers
Copy link
Contributor

Change to PR detected. A new PR build was completed.
A new image has been built to help with testing out this PR: quay.io/opendatahub/data-science-pipelines-operator:pr-440

@HumairAK HumairAK marked this pull request as ready for review November 1, 2023 17:31
@HumairAK HumairAK added the qe/verify Labels to inform qe on issues to verify. label Nov 1, 2023
@HumairAK HumairAK changed the title Issue 244 Add CA Injection into DSP via DSPA Nov 1, 2023
@HumairAK HumairAK changed the title Add CA Injection into DSP via DSPA Add Custom CA Bundle Injection into DSP via DSPA Nov 1, 2023
Copy link
Contributor

@rimolive rimolive left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

Copy link
Contributor

openshift-ci bot commented Nov 2, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rimolive

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:

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

@openshift-ci openshift-ci bot added the approved label Nov 2, 2023
@HumairAK
Copy link
Contributor Author

HumairAK commented Nov 2, 2023

/hold

@rimolive
Copy link
Contributor

rimolive commented Nov 2, 2023

/hold

@HumairAK
Copy link
Contributor Author

HumairAK commented Nov 2, 2023

@gregsheremeta

@gregsheremeta
Copy link
Contributor

gregsheremeta commented Nov 3, 2023

Check DSPO logs, you should see an error mentioning x509 and instructions on adding a cabundle.

I didn't see this. The first thing that happens is that the health check fails.

2023-11-03T18:24:34Z INFO Performing Object Storage Health Check {"namespace": "dspa1", "dspa_name": "dspa1"}
2023-11-03T18:24:34Z INFO Could not connect to object storage endpoint: https://minio-api-minioself.apps.rosa.greg-1002.xb4x.p3.openshiftapps.com {"namespace": "dspa1", "dspa_name": "dspa1"}
2023-11-03T18:24:34Z INFO Object Storage Health Check Failed {"namespace": "dspa1", "dspa_name": "dspa1"}
2023-11-03T18:24:34Z INFO Updating CR status {"namespace": "dspa1", "dspa_name": "dspa1"}

Searching the logs for the word "cabundle" turns up empty.

Need to add something like this to the health check flow too?

edit: I was hitting a separate known issue where externalStorage.host cannot start with https://

@gregsheremeta
Copy link
Contributor

verified. Pipeline artifacts are written to my self-signed minio. Feel free to unhold

@HumairAK
Copy link
Contributor Author

HumairAK commented Nov 6, 2023

/unhold

@openshift-merge-bot openshift-merge-bot bot merged commit 6206cf2 into opendatahub-io:main Nov 6, 2023
5 checks passed
@shalberd
Copy link
Contributor

shalberd commented Nov 7, 2023

mmh, it would have been good if that were to be using the central Cluster additionalCA and system CA bundle, via configmap, as done in Notebooks. Now here, you have another configmap where the PEM content needs to be in.

see https://github.com/opendatahub-io/kubeflow/pull/43/files#diff-447c1a1ddad4e46669c4371d0d9714dad4a3368c5ccf2292b356e3b5c7441ce1

Essentially, in notebooks, the odh operator does the creation of the secret in the namespace and the mapping of its content into the notebook container volume automatically without defining the secret by name in OdhDashboardConfig.

#362 (comment)

Your approach is to define the secret in DataSciencePipelines CR, to reference any configmap by name via CR configmapKey. Which is fine, but now, there is additional config settings work. The advantage: You way, you can link up any secret even if you don't have access to Cluster Proxy Config.

@gregsheremeta
Copy link
Contributor

The advantage would be that I only have to add custom trusted CAs and self-signed certs in PEM format once at central cluster Proxy config in a secret in namespace openshift-config, which is much more streamlined and less distributed.

Long term, we are pursuing a global cluster-level approach. Whether that uses the Proxy or not is TBD.

A future global cluster-level approach does not obviate the need for component-level control as well.

@shalberd
Copy link
Contributor

shalberd commented Nov 7, 2023

First of all: it is great that one is able to configure custom CAs and self-signed certs in DSPA CR now, extremely helpful, thank you for the hard work.

Long term, we are pursuing a global cluster-level approach. Whether that uses the Proxy or not is TBD.

cluster-level Proxy config for CAs is just a gathering point for publicly-trusted CAs that come with Openshift and don't have to be configured plus then additional trusted CAs that are not publicly trusted or even just self-signed certs.

A future global cluster-level approach does not obviate the need for component-level control as well.

Agreed: in case customers use third-party operators to collect and inject CAs, the flow could be: use Proxy config CA-bundle if nothing is defined in a field in the component config CR at component-level; good idea and approach.

By the way: the CAs and self-signed certs defined in proxy config and injectable together with core Red Hat OS based publicly trusted CA via configmap via the cluster network operator at cluster level can be applied and used as a source of trust for any connection, regardless of whether they go over HTTP_PROXY targets or NO_PROXY targets.

Similar to how in detail imagestreams work, I sometimes get the impression that the possibility to add own certs and CAs via Proxy config is not very well-known and documented by RedHat, especially that all-in-one bundle (public system plus own additional) injection mechanism.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm qe/verify Labels to inform qe on issues to verify.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Pipeline server fails when using object storage "signed by unknown authority"
5 participants