feat: add tls support for apiserver http/grpc #40
Conversation
|
A set of new images have been built to help with testing out this PR: |
|
An OCP cluster where you are logged in as cluster admin is required. The Data Science Pipelines team recommends testing this using the Data Science Pipelines Operator. Check here for more information on using the DSPO. To use and deploy a DSP stack with these images (assuming the DSPO is deployed), first save the following YAML to a file named apiVersion: datasciencepipelinesapplications.opendatahub.io/v1alpha1
kind: DataSciencePipelinesApplication
metadata:
name: pr-40
spec:
dspVersion: v2
apiServer:
image: "quay.io/opendatahub/ds-pipelines-api-server:pr-40"
argoDriverImage: "quay.io/opendatahub/ds-pipelines-driver:pr-40"
argoLauncherImage: "quay.io/opendatahub/ds-pipelines-launcher:pr-40"
persistenceAgent:
image: "quay.io/opendatahub/ds-pipelines-persistenceagent:pr-40"
scheduledWorkflow:
image: "quay.io/opendatahub/ds-pipelines-scheduledworkflow:pr-40"
mlmd:
deploy: true # Optional component
grpc:
image: "quay.io/opendatahub/mlmd-grpc-server:latest"
envoy:
image: "registry.redhat.io/openshift-service-mesh/proxyv2-rhel8:2.3.9-2"
mlpipelineUI:
deploy: true # Optional component
image: "quay.io/opendatahub/ds-pipelines-frontend:pr-40"
objectStorage:
minio:
deploy: true
image: 'quay.io/opendatahub/minio:RELEASE.2019-08-14T20-37-41Z-license-compliance'Then run the following: cd $(mktemp -d)
git clone [email protected]:opendatahub-io/data-science-pipelines.git
cd data-science-pipelines/
git fetch origin pull/40/head
git checkout -b pullrequest 03d741462224bad67faa17c71259fc4f9d5b025d
oc apply -f dspa.pr-40.yamlMore instructions here on how to deploy and test a Data Science Pipelines Application. |
Commit Checker results: |
|
Change to PR detected. A new PR build was completed. |
HumairAK
changed the title
Commit Checker results: |
|
Change to PR detected. A new PR build was completed. |
|
would additionally suggest adding a unit test here to verify this functionality. |
Commit Checker results: |
|
Change to PR detected. A new PR build was completed. |
testing tls connections via unit testing is going to be non trivial, I think this is best reserved for integration testing |
Commit Checker results: |
|
Change to PR detected. A new PR build was completed. |
HumairAK
requested review from
gregsheremeta and
DharmitD
and removed request for
amadhusu
|
/lgtm just for my own learning, why'd you flip the flag from bool to string? |
The reason is because flag.bool requires you to be very precise in how you declare the boolean it has to be like at some point we should just do away with golang default flags and use viper |
|
I can tag this with approve once @VaniHaripriya verifies opendatahub-io/data-science-pipelines-operator#656 |
|
/verified |
VaniHaripriya
added
the
verified
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gregsheremeta 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 |
|
oops just noticed the conflict |
make mlpipeline server url scheme configurable add tls handling for PA and ui remove local grpc client tls. Signed-off-by: Humair Khan <[email protected]>
Commit Checker results: |
|
/lgtm |
Commit Checker results: |
|
Change to PR detected. A new PR build was completed. |
Resolves: https://issues.redhat.com/browse/RHOAIENG-4968
Depends on: opendatahub-io/data-science-pipelines-operator#656
See DSPO pr for testing instructions