Move IAP config to initContainer on the envoy pod #457
Conversation
|
/assign @ankushagarwal |
|
@danisla Can you pull in the latest changes and resolve the conflicts? |
|
@ankushagarwal done, I rebased and pushed the merged changes. |
| @@ -8,9 +8,14 @@ | |||
| // @param ipName string The name of the global ip address to use. | |||
| // @optionalParam hostname string null The hostname associated with this ingress. Eg: mykubeflow.example.com | |||
| // @optionalParam issuer string letsencrypt-prod The cert-manager issuer name. | |||
| // @optionalParam envoyImage string gcr.io/kubeflow-images-staging/envoy:v20180309-0fb4886b463698702b6a08955045731903a18738 The image for envoy. | |||
| // @optionalParam disableJwtChecking string false Disable JWT checking. | |||
| // @param clientID string OAuth client ID. | |||
There was a problem hiding this comment.
nit: Can you move all optionalParams to the bottom
|
|
||
| NODE_PORT=$(kubectl --namespace=${NAMESPACE} get svc ${SERVICE} -o jsonpath='{.spec.ports[0].nodePort}') | ||
| while [[ -z ${BACKEND_ID} ]]; | ||
| do BACKEND_ID=$(gcloud compute --project=${PROJECT} backend-services list --filter=name~k8s-be-${NODE_PORT}- --format='value(id)'); |
There was a problem hiding this comment.
Does this use the Compute Engine default service account credentials? If so, can you add a comment in the iap.md document to mention that "Compute Engine default service account" should have Project Editor role or higher in IAM settings.
There was a problem hiding this comment.
It will use the service account for the GKE cluster, which by default is the Compute Engine default service account. I'll add a note about the permission required in the iap.md.
kubeflow/core/iap.libsonnet
Outdated
|
|
||
| JWT_AUDIENCE="/projects/${PROJECT_NUM}/global/backendServices/${BACKEND_ID}" | ||
|
|
||
| [ -z ${JWT_AUDIENCE} ] && echo "Error JWT_AUDIENCE couldn't be set" && exit 1 |
There was a problem hiding this comment.
JWT_AUDIENCE will never have zero length - so this check is redundant.
|
|
||
| ``` | ||
| ks generate cert-manager cert-manager --acmeEmail=${ACCOUNT} | ||
| ks apply ${ENVIRONMENT} -c cert-manager | ||
|
|
||
| ks generate iap-ingress iap-ingress --ipName=${IP_NAME} --hostname=${FQDN} | ||
| ks generate iap-ingress iap-ingress --namespace=${NAMESPACE} --ipName=${IP_NAME} --hostname=${FQDN} --clientID=${CLIENT_ID} --clientSecret=${CLIENT_SECRET} |
There was a problem hiding this comment.
nit: namespace is not needed as it is automatically inherited from ks env namespace.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ankushagarwal 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 |
* Adding PeriodSeconds to livenessProbe * Add tests * Adding extra kubectl output
* Adding initial v1alpha2 controller * Adding logs * Adding comments * Adding template functions for experiment * Adding error checks
Signed-off-by: ted chang <htchang@us.ibm.com>
This automates the steps needed to enable IAP on the ingress and the need for the 2-step envoy deployment.
The
enable_iap.shscript is now run as an initContainer on the envoy pod. A distributed lock is used on the envoy service annotation to ensure the script is not run concurrently.This change is