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

Use DSPA custom ca cert on MLMD and Persistence Agent clients #736

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

hbelmiro
Copy link
Contributor

@hbelmiro hbelmiro commented Oct 31, 2024

This PR must be merged with opendatahub-io/data-science-pipelines#96

The issue resolved by this Pull Request:

Resolves https://issues.redhat.com/browse/RHOAIENG-13871

Description of your changes:

This PR adds validation of MLMD and Persistence Agent server certificates against CA in the clients.

Testing instructions

There are 2 scenarios:

With podToPodTLS: true

  1. Deploy the following DSPA:
apiVersion: datasciencepipelinesapplications.opendatahub.io/v1
kind: DataSciencePipelinesApplication
metadata:
  name: sample
spec:
  dspVersion: v2
  apiServer:
    enableSamplePipeline: true
    podToPodTLS: true
    image: quay.io/opendatahub/ds-pipelines-api-server:pr-96
    argoDriverImage: quay.io/opendatahub/ds-pipelines-driver:pr-96
    argoLauncherImage: quay.io/opendatahub/ds-pipelines-launcher:pr-96
    cABundle:
      configMapKey: ca.crt
      configMapName: kube-root-ca.crt
  persistenceAgent:
    deploy: true
    image: quay.io/opendatahub/ds-pipelines-persistenceagent:pr-96
    numWorkers: 2
    resources:
      requests:
        cpu: 120m
        memory: 500Mi
      limits:
        cpu: 250m
        memory: 1Gi
  objectStorage:
    # Need to enable this for artifact download links to work
    # i.e. for when requesting /apis/v2beta1/artifacts/{id}?share_url=true
    enableExternalRoute: true
    minio:
      deploy: true
      image: 'quay.io/opendatahub/minio:RELEASE.2019-08-14T20-37-41Z-license-compliance'
  mlpipelineUI:
    image: quay.io/opendatahub/ds-pipelines-frontend:latest
  1. Run the iris pipeline
  2. The run must complete successfully

With podToPodTLS: false

  1. Deploy the following DSPA:
apiVersion: datasciencepipelinesapplications.opendatahub.io/v1
kind: DataSciencePipelinesApplication
metadata:
  name: sample
spec:
  dspVersion: v2
  apiServer:
    enableSamplePipeline: true
    podToPodTLS: false
    image: quay.io/opendatahub/ds-pipelines-api-server:pr-96
    argoDriverImage: quay.io/opendatahub/ds-pipelines-driver:pr-96
    argoLauncherImage: quay.io/opendatahub/ds-pipelines-launcher:pr-96
    cABundle:
      configMapKey: ca.crt
      configMapName: kube-root-ca.crt
  persistenceAgent:
    deploy: true
    image: quay.io/opendatahub/ds-pipelines-persistenceagent:pr-96
    numWorkers: 2
    resources:
      requests:
        cpu: 120m
        memory: 500Mi
      limits:
        cpu: 250m
        memory: 1Gi
  objectStorage:
    # Need to enable this for artifact download links to work
    # i.e. for when requesting /apis/v2beta1/artifacts/{id}?share_url=true
    enableExternalRoute: true
    minio:
      deploy: true
      image: 'quay.io/opendatahub/minio:RELEASE.2019-08-14T20-37-41Z-license-compliance'
  mlpipelineUI:
    image: quay.io/opendatahub/ds-pipelines-frontend:latest
  1. Run the iris pipeline
  2. The run must complete successfully

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

Copy link
Contributor

openshift-ci bot commented Oct 31, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copy link
Contributor

openshift-ci bot commented Oct 31, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign dharmitd for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@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-736
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 [email protected]:opendatahub-io/data-science-pipelines-operator.git
cd data-science-pipelines-operator/
git fetch origin pull/736/head
git checkout -b pullrequest 22c4d79335e2268a82cd1cd68bd844a7a4821ccf
oc new-project opendatahub
make deploy IMG="quay.io/opendatahub/data-science-pipelines-operator:pr-736"

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

@hbelmiro hbelmiro changed the title WIP - Use DSPA custom ca cert on MLMD and Persistence Agent clients Use DSPA custom ca cert on MLMD and Persistence Agent clients Oct 31, 2024
@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-736

@hbelmiro hbelmiro marked this pull request as ready for review October 31, 2024 18:50
@hbelmiro
Copy link
Contributor Author

hbelmiro commented Nov 1, 2024

/hold
While this PR has images hardcoded in params.env for testing.

@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-736

@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-736

@hbelmiro hbelmiro marked this pull request as draft November 1, 2024 18:32
@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-736

2 similar comments
@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-736

@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-736

@hbelmiro hbelmiro marked this pull request as ready for review November 4, 2024 14:08
@openshift-ci openshift-ci bot requested a review from VaniHaripriya November 4, 2024 14:08
@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-736

@diegolovison
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot removed the lgtm label Nov 4, 2024
Copy link
Contributor

openshift-ci bot commented Nov 4, 2024

New changes are detected. LGTM label has been removed.

@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-736

@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-736

@HumairAK HumairAK merged commit 8553b62 into opendatahub-io:main Nov 4, 2024
6 of 7 checks passed
@hbelmiro hbelmiro deleted the RHOAIENG-13871 branch November 4, 2024 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants