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

(feat): add tls to api server #656

Merged
merged 2 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions api/v1alpha1/dspipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ type DSPASpec struct {
// +kubebuilder:validation:Optional
// +kubebuilder:default:="v1"
DSPVersion string `json:"dspVersion,omitempty"`

// PodToPodTLS Set to "true" or "false" to enable or disable TLS communication between DSPA components (pods). Defaults to "true" to enable TLS between all pods. Only supported in DSP V2 on OpenShift.
// +kubebuilder:default:=true
// +kubebuilder:validation:Optional
PodToPodTLS *bool `json:"podToPodTLS"`

// WorkflowController is an argo-specific component that manages a DSPA's Workflow objects and handles the orchestration of them with the central Argo server
// +kubebuilder:validation:Optional
*WorkflowController `json:"workflowController,omitempty"`
Expand Down
5 changes: 5 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,12 @@ spec:
type: object
type: object
type: object
podToPodTLS:
default: true
description: PodToPodTLS Set to "true" or "false" to enable or disable
TLS communication between DSPA components (pods). Defaults to "true"
to enable TLS between all pods. Only supported in DSP V2 on OpenShift.
type: boolean
scheduledWorkflow:
default:
deploy: true
Expand Down
47 changes: 31 additions & 16 deletions config/internal/apiserver/default/deployment.yaml.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ spec:
value: "8887"
- name: SIGNED_URL_EXPIRY_TIME_SECONDS
value: "{{.APIServer.ArtifactSignedURLExpirySeconds}}"
{{ if .PodToPodTLS }}
- name: ML_PIPELINE_TLS_ENABLED
value: "true"
{{ end }}
{{ if (eq .DSPVersion "v2") }}
## Argo-Specific Env Vars ##
- name: EXECUTIONTYPE
Expand Down Expand Up @@ -181,32 +185,32 @@ spec:
{{ if .APIServer.EnableSamplePipeline }}
- --sampleconfig=/config/sample_config.json
{{ end }}
{{ if .PodToPodTLS }}
- --tlsCertPath=/etc/tls/private/tls.crt
- --tlsCertKeyPath=/etc/tls/private/tls.key
gregsheremeta marked this conversation as resolved.
Show resolved Hide resolved
{{ end }}
ports:
- containerPort: 8888
name: http
- containerPort: 8887
name: grpc
livenessProbe:
exec:
command:
- wget
- -q
- -S
- -O
- '-'
- http://localhost:8888/apis/v1beta1/healthz
httpGet:
path: /apis/v1beta1/healthz
port: http
{{ if .PodToPodTLS }}
scheme: HTTPS
{{ end }}
initialDelaySeconds: 3
periodSeconds: 5
timeoutSeconds: 2
readinessProbe:
exec:
command:
- wget
- -q
- -S
- -O
- '-'
- http://localhost:8888/apis/v1beta1/healthz
httpGet:
path: /apis/v1beta1/healthz
port: http
{{ if .PodToPodTLS }}
scheme: HTTPS
{{ end }}
initialDelaySeconds: 3
periodSeconds: 5
timeoutSeconds: 2
Expand All @@ -233,6 +237,10 @@ spec:
- name: server-config
mountPath: /config/config.json
subPath: {{ .APIServer.CustomServerConfig.Key }}
{{ if .PodToPodTLS }}
- mountPath: /etc/tls/private
name: proxy-tls
{{ end }}
{{ if or .APIServer.EnableSamplePipeline .CustomCABundle }}
{{ if .APIServer.EnableSamplePipeline }}
- name: sample-config
Expand All @@ -252,7 +260,14 @@ spec:
- --https-address=:8443
- --provider=openshift
- --openshift-service-account={{.APIServerDefaultResourceName}}
{{ if .PodToPodTLS }}
# because we use certs signed by openshift, these certs are not valid for
# localhost, thus we have to use the service name
- --upstream=https://{{.APIServerServiceDNSName}}:8888
gregsheremeta marked this conversation as resolved.
Show resolved Hide resolved
- --upstream-ca=/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt
{{ else }}
- --upstream=http://localhost:8888
{{ end }}
- --tls-cert=/etc/tls/private/tls.crt
- --tls-key=/etc/tls/private/tls.key
- --cookie-secret=SECRET
Expand Down
8 changes: 7 additions & 1 deletion config/internal/mlpipelines-ui/deployment.yaml.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,15 @@ spec:
- name: ARGO_ARCHIVE_LOGS
value: "true"
- name: ML_PIPELINE_SERVICE_HOST
value: ds-pipeline-{{.Name}}
value: {{.APIServerServiceDNSName}}
- name: ML_PIPELINE_SERVICE_PORT
value: '8888'
{{ if .PodToPodTLS }}
- name: ML_PIPELINE_SERVICE_SCHEME
value: 'https'
- name: NODE_EXTRA_CA_CERTS
value: '/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt'
{{ end }}
- name: METADATA_ENVOY_SERVICE_SERVICE_HOST
value: ds-pipeline-md-{{.Name}}
- name: METADATA_ENVOY_SERVICE_SERVICE_PORT
Expand Down
9 changes: 8 additions & 1 deletion config/internal/persistence-agent/deployment.yaml.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ spec:
{{ else }}
value: PipelineRun
{{ end }}
{{ if .PodToPodTLS }}
- name: SSL_CERT_DIR
value: "/etc/pki/tls/certs:/var/run/secrets/kubernetes.io/serviceaccount/"
{{ end }}
image: "{{.PersistenceAgent.Image}}"
imagePullPolicy: IfNotPresent
name: ds-pipeline-persistenceagent
Expand All @@ -48,7 +52,10 @@ spec:
- "--logtostderr=true"
- "--ttlSecondsAfterWorkflowFinish=86400"
- "--numWorker={{.PersistenceAgent.NumWorkers}}"
- "--mlPipelineAPIServerName={{.APIServerServiceName}}"
- "--mlPipelineAPIServerName={{.APIServerServiceDNSName}}"
{{ if .PodToPodTLS }}
- "--mlPipelineServiceTLSEnabled=true"
{{ end }}
- "--namespace={{.Namespace}}"
- "--mlPipelineServiceHttpPort=8888"
- "--mlPipelineServiceGRPCPort=8887"
Expand Down
3 changes: 3 additions & 0 deletions controllers/config/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ const (
CustomDSPTrustedCAConfigMapNamePrefix = "dsp-trusted-ca"
CustomDSPTrustedCAConfigMapKey = "dsp-ca.crt"

OpenshiftServiceCAConfigMapName = "openshift-service-ca.crt"
OpenshiftServiceCAConfigMapKey = "service-ca.crt"

DefaultSystemSSLCertFile = "SSL_CERT_FILE"
DefaultSystemSSLCertFilePath = "/etc/pki/tls/certs/ca-bundle.crt" // Fedora/RHEL 6

Expand Down
38 changes: 35 additions & 3 deletions controllers/dspipeline_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ type DSPAParams struct {
// pipeline pods
CustomCABundle *dspa.CABundle
DSPONamespace string
// Use to enable tls communication between component pods.
PodToPodTLS bool

APIServerServiceDNSName string
}

type DBConnection struct {
Expand Down Expand Up @@ -578,6 +582,7 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
p.APIServer = dsp.Spec.APIServer.DeepCopy()
p.APIServerDefaultResourceName = apiServerDefaultResourceNamePrefix + dsp.Name
p.APIServerServiceName = fmt.Sprintf("%s-%s", config.DSPServicePrefix, p.Name)
p.APIServerServiceDNSName = fmt.Sprintf("%s.%s.svc.cluster.local", p.APIServerServiceName, p.Namespace)
gregsheremeta marked this conversation as resolved.
Show resolved Hide resolved
p.ScheduledWorkflow = dsp.Spec.ScheduledWorkflow.DeepCopy()
p.ScheduledWorkflowDefaultResourceName = scheduledWorkflowDefaultResourceNamePrefix + dsp.Name
p.PersistenceAgent = dsp.Spec.PersistenceAgent.DeepCopy()
Expand All @@ -589,8 +594,19 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
p.MLMD = dsp.Spec.MLMD.DeepCopy()
p.CustomCABundleRootMountPath = config.CustomCABundleRootMountPath
p.PiplinesCABundleMountPath = config.GetCABundleFileMountPath()
p.PodToPodTLS = false
dspTrustedCAConfigMapKey := config.CustomDSPTrustedCAConfigMapKey

// PodToPodTLS is only used in v2 dsp
if p.UsingV2Pipelines(dsp) {
// by default it's enabled when omitted
if dsp.Spec.PodToPodTLS == nil {
p.PodToPodTLS = true
} else {
p.PodToPodTLS = *dsp.Spec.PodToPodTLS
}
}

log := loggr.WithValues("namespace", p.Namespace).WithValues("dspa_name", p.Name)

if p.APIServer != nil {
Expand Down Expand Up @@ -633,7 +649,7 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
// Track whether the "ca-bundle.crt" configmap key from odh-trusted-ca bundle
// was found, this will be used to decide whether we need to account for this
// ourselves later or not.
odhTrustedCABundleAdded := false
wellKnownCABundleAdded := false

// Check for cert bundle provided by the platform instead of by the DSPA user
// If it exists, include this cert for tls verifications
Expand Down Expand Up @@ -661,7 +677,7 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
// however if a user creates this, they may accidentally leave this out, so we need to account for this
_, ok := odhTrustedCABundleConfigMap.Data[config.GlobalODHCaBundleConfigMapSystemBundleKey]
if ok {
odhTrustedCABundleAdded = true
wellKnownCABundleAdded = true
}
}

Expand All @@ -683,6 +699,22 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
}
}

// If PodToPodTLS is enabled, we need to include service-ca ca-bundles to recognize the certs
// that are signed by service-ca. These can be accessed via "openshift-service-ca.crt"
// configmap.
if p.PodToPodTLS {
serviceCA, serviceCACfgErr := util.GetConfigMap(ctx, config.OpenshiftServiceCAConfigMapName, p.Namespace, client)
if serviceCACfgErr != nil {
log.Info(fmt.Sprintf("Encountered error when attempting to fetch ConfigMap: [%s]. Error: %v", config.OpenshiftServiceCAConfigMapName, serviceCA))
return serviceCACfgErr
}
serviceCABundle := util.GetConfigMapValue(config.OpenshiftServiceCAConfigMapKey, serviceCA)
if serviceCABundle == "" {
return fmt.Errorf("expected key %s from configmap %s not found", config.OpenshiftServiceCAConfigMapKey, config.OpenshiftServiceCAConfigMapName)
}
p.APICustomPemCerts = append(p.APICustomPemCerts, []byte(serviceCABundle))
}

if p.APIServer.CABundleFileMountPath != "" {
p.CustomCABundleRootMountPath = p.APIServer.CABundleFileMountPath
}
Expand All @@ -706,7 +738,7 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
// We need to ensure system certs are always part of this new configmap
// We can either source this from odh-trusted-ca-bundle cfgmap if provided,
// or fetch one from "config-trusted-cabundle" configmap, which is always present in an ocp ns
if !odhTrustedCABundleAdded {
if !wellKnownCABundleAdded {
certs, sysCertsErr := util.GetSystemCerts()
if sysCertsErr != nil {
return sysCertsErr
Expand Down
42 changes: 37 additions & 5 deletions controllers/dspipeline_params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,38 @@ func TestExtractParams_CABundle(t *testing.T) {
},
SSLCertFileEnv: "testdata/tls/dummy-ca-bundle.crt",
},

{
msg: "pod to pod tls enabled",
dsp: testutil.CreateDSPAWithAPIServerPodtoPodTlsEnabled(),
CustomCABundleRootMountPath: "/dsp-custom-certs",
CustomSSLCertDir: strPtr("/dsp-custom-certs:/etc/ssl/certs:/etc/pki/tls/certs"),
PiplinesCABundleMountPath: "/dsp-custom-certs/dsp-ca.crt",
APICustomPemCerts: [][]byte{[]byte("service-ca-contents")},
CustomCABundle: &dspav1alpha1.CABundle{ConfigMapKey: "dsp-ca.crt", ConfigMapName: "dsp-trusted-ca-testdspa"},
ConfigMapPreReq: []*v1.ConfigMap{
{
ObjectMeta: metav1.ObjectMeta{Name: "openshift-service-ca.crt", Namespace: "testnamespace"},
Data: map[string]string{"service-ca.crt": "service-ca-contents"},
},
},
},
{
msg: "pod to pod tls enabled with sys certs",
dsp: testutil.CreateDSPAWithAPIServerPodtoPodTlsEnabled(),
CustomCABundleRootMountPath: "/dsp-custom-certs",
CustomSSLCertDir: strPtr("/dsp-custom-certs:/etc/ssl/certs:/etc/pki/tls/certs"),
PiplinesCABundleMountPath: "/dsp-custom-certs/dsp-ca.crt",
APICustomPemCerts: [][]byte{[]byte("service-ca-contents"), []byte("dummycontent")},
CustomCABundle: &dspav1alpha1.CABundle{ConfigMapKey: "dsp-ca.crt", ConfigMapName: "dsp-trusted-ca-testdspa"},
ConfigMapPreReq: []*v1.ConfigMap{
{
ObjectMeta: metav1.ObjectMeta{Name: "openshift-service-ca.crt", Namespace: "testnamespace"},
Data: map[string]string{"service-ca.crt": "service-ca-contents"},
},
},
SSLCertFileEnv: "testdata/tls/dummy-ca-bundle.crt",
},
}

for _, test := range tt {
Expand All @@ -199,19 +231,19 @@ func TestExtractParams_CABundle(t *testing.T) {
}

actualCustomCABundleRootMountPath := actualParams.CustomCABundleRootMountPath
assert.Equal(t, actualCustomCABundleRootMountPath, test.CustomCABundleRootMountPath)
assert.Equal(t, test.CustomCABundleRootMountPath, actualCustomCABundleRootMountPath)

actualCustomSSLCertDir := actualParams.CustomSSLCertDir
assert.Equal(t, actualCustomSSLCertDir, test.CustomSSLCertDir)
assert.Equal(t, test.CustomSSLCertDir, actualCustomSSLCertDir)

actualPipelinesCABundleMountPath := actualParams.PiplinesCABundleMountPath
assert.Equal(t, actualPipelinesCABundleMountPath, test.PiplinesCABundleMountPath)
assert.Equal(t, test.PiplinesCABundleMountPath, actualPipelinesCABundleMountPath)

actualAPICustomPemCerts := actualParams.APICustomPemCerts
assert.Equal(t, actualAPICustomPemCerts, test.APICustomPemCerts)
assert.Equal(t, test.APICustomPemCerts, actualAPICustomPemCerts)

actualCustomCABundle := actualParams.CustomCABundle
assert.Equal(t, actualCustomCABundle, test.CustomCABundle)
assert.Equal(t, test.CustomCABundle, actualCustomCABundle)

if test.ConfigMapPreReq != nil && len(test.ConfigMapPreReq) > 0 {
for _, cfg := range test.ConfigMapPreReq {
Expand Down
29 changes: 19 additions & 10 deletions controllers/mlmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,9 @@ func TestDeployMLMDV2(t *testing.T) {
// Construct DSPA Spec with MLMD Enabled
dspa := &dspav1alpha1.DataSciencePipelinesApplication{
Spec: dspav1alpha1.DSPASpec{
DSPVersion: "v2",
APIServer: &dspav1alpha1.APIServer{},
DSPVersion: "v2",
PodToPodTLS: boolPtr(false),
HumairAK marked this conversation as resolved.
Show resolved Hide resolved
APIServer: &dspav1alpha1.APIServer{},
MLMD: &dspav1alpha1.MLMD{
Deploy: true,
},
Expand Down Expand Up @@ -315,8 +316,9 @@ func TestDontDeployMLMDV2(t *testing.T) {
// Construct DSPA Spec with MLMD Not Enabled
dspa := &dspav1alpha1.DataSciencePipelinesApplication{
Spec: dspav1alpha1.DSPASpec{
DSPVersion: "v2",
APIServer: &dspav1alpha1.APIServer{},
DSPVersion: "v2",
PodToPodTLS: boolPtr(false),
APIServer: &dspav1alpha1.APIServer{},
MLMD: &dspav1alpha1.MLMD{
Deploy: false,
},
Expand Down Expand Up @@ -448,8 +450,9 @@ func TestDefaultDeployBehaviorMLMDV2(t *testing.T) {
// Construct DSPA Spec with MLMD Spec not defined
dspa := &dspav1alpha1.DataSciencePipelinesApplication{
Spec: dspav1alpha1.DSPASpec{
DSPVersion: "v2",
APIServer: &dspav1alpha1.APIServer{},
DSPVersion: "v2",
PodToPodTLS: boolPtr(false),
APIServer: &dspav1alpha1.APIServer{},
Database: &dspav1alpha1.Database{
DisableHealthCheck: false,
MariaDB: &dspav1alpha1.MariaDB{
Expand Down Expand Up @@ -608,8 +611,9 @@ func TestDeployEnvoyRouteV2(t *testing.T) {
// Construct DSPA Spec with MLMD Enabled
dspa := &dspav1alpha1.DataSciencePipelinesApplication{
Spec: dspav1alpha1.DSPASpec{
DSPVersion: "v2",
APIServer: &dspav1alpha1.APIServer{},
DSPVersion: "v2",
PodToPodTLS: boolPtr(false),
APIServer: &dspav1alpha1.APIServer{},
MLMD: &dspav1alpha1.MLMD{
Deploy: true,
Envoy: &dspav1alpha1.Envoy{
Expand Down Expand Up @@ -750,8 +754,9 @@ func TestDontDeployEnvoyRouteV2(t *testing.T) {
// Construct DSPA Spec with MLMD Enabled
dspa := &dspav1alpha1.DataSciencePipelinesApplication{
Spec: dspav1alpha1.DSPASpec{
DSPVersion: "v2",
APIServer: &dspav1alpha1.APIServer{},
DSPVersion: "v2",
PodToPodTLS: boolPtr(false),
APIServer: &dspav1alpha1.APIServer{},
MLMD: &dspav1alpha1.MLMD{
Deploy: true,
Envoy: &dspav1alpha1.Envoy{
Expand Down Expand Up @@ -811,3 +816,7 @@ func TestDontDeployEnvoyRouteV2(t *testing.T) {
assert.False(t, created)
assert.Nil(t, err)
}

func boolPtr(b bool) *bool {
return &b
}
Loading
Loading