Make basic auth works with ISTIO - JSONNET (kubeflow#3404)

* use ISTIO for basic auth e2e tests * add dummy line to trigger tests * add presubmit * fix update_backend * fix setup_backend * add env * update naming * switch to istio-system * add injectIstio to basic auth ing * move ambassador to istio-system * add injectIstio to ambassador * use temp controller * fix service * add link from ambassador to istio * use regex * fix prefix * remove precedence * revert image * add missing param to ambassador test * change ambassador test to use istio * remove dummy * update comment * add comment
ricardov1 · Jun 7, 2019 · 91ccc4c · 91ccc4c
1 parent df572ce
commit 91ccc4c
Show file tree

Hide file tree

Showing 10 changed files with 99 additions and 24 deletions.
diff --git a/bootstrap/config/overlays/basic_auth/kfctl_default.yaml b/bootstrap/config/overlays/basic_auth/kfctl_default.yaml
@@ -7,6 +7,9 @@ spec:
     ambassador:
     - name: ambassadorServiceType
       value: NodePort
+    - initRequired: true
+      name: injectIstio
+      value: "false"
     argo:
     - initRequired: true
       name: injectIstio
@@ -36,6 +39,9 @@ spec:
       value: envoy-ingress
     - name: issuer
       value: letsencrypt-prod
+    - initRequired: true
+      name: injectIstio
+      value: "false"
     notebook-controller:
     - name: injectGcpCredentials
       value: "true"

diff --git a/kubeflow/common/ambassador.libsonnet b/kubeflow/common/ambassador.libsonnet
@@ -2,7 +2,10 @@
   local k = import "k.libsonnet",
   local util = import "kubeflow/common/util.libsonnet",
   new(_env, _params):: {
-    local params = _params + _env,
+    local params = _params + _env {
+      injectIstio: util.toBool(_params.injectIstio),
+    },
+    local namespace = if params.injectIstio then params.istioNamespace else params.namespace,
 
     local ambassadorService = {
       apiVersion: "v1",
@@ -12,7 +15,7 @@
           service: "ambassador",
         },
         name: "ambassador",
-        namespace: params.namespace,
+        namespace: namespace,
       },
       spec: {
         ports: [
@@ -42,7 +45,7 @@
           service: "ambassador-admin",
         },
         name: "ambassador-admin",
-        namespace: params.namespace,
+        namespace: namespace,
       },
       spec: {
         ports: [
@@ -118,7 +121,7 @@
       kind: "ServiceAccount",
       metadata: {
         name: "ambassador",
-        namespace: params.namespace,
+        namespace: namespace,
       },
     },  // serviceAccount
     ambassadorServiceAccount:: ambassadorServiceAccount,
@@ -138,7 +141,7 @@
         {
           kind: "ServiceAccount",
           name: "ambassador",
-          namespace: params.namespace,
+          namespace: namespace,
         },
       ],
     },  // roleBinding
@@ -149,7 +152,7 @@
       kind: "Deployment",
       metadata: {
         name: "ambassador",
-        namespace: params.namespace,
+        namespace: namespace,
       },
       spec: {
         replicas: params.replicas,
@@ -158,7 +161,7 @@
             labels: {
               service: "ambassador",
             },
-            namespace: params.namespace,
+            namespace: namespace,
           },
           spec: {
             containers: [

diff --git a/kubeflow/common/prototypes/ambassador.jsonnet b/kubeflow/common/prototypes/ambassador.jsonnet
@@ -8,6 +8,8 @@
 // @optionalParam ambassadorNodePort number 0 Optional nodePort to use when ambassadorServiceType is NodePort {30000-32767}.
 // @optionalParam ambassadorImage string quay.io/datawire/ambassador:0.37.0 The image for the API Gateway.
 // @optionalParam replicas number 3 The number of replicas.
+// @optionalParam injectIstio string false Whether to use ISTIO.
+// @optionalParam istioNamespace string istio-system The namespace where Istio is installed
 
 local ambassador = import "kubeflow/common/ambassador.libsonnet";
 local instance = ambassador.new(env, params);

diff --git a/kubeflow/common/tests/ambassador_test.jsonnet b/kubeflow/common/tests/ambassador_test.jsonnet
@@ -7,6 +7,8 @@ local params = {
   ambassadorServiceType: "ClusterIP",
   ambassadorImage: "quay.io/datawire/ambassador:0.37.0",
   replicas: 3,
+  injectIstio: true,
+  istioNamespace: "istio-test",
 };
 local env = {
   namespace: "kubeflow",
@@ -26,7 +28,7 @@ local testCases = [
             service: "ambassador",
           },
           name: "ambassador",
-          namespace: "kubeflow",
+          namespace: "istio-test",
         },
         spec: {
           ports: [
@@ -53,7 +55,7 @@ local testCases = [
           service: "ambassador-admin",
         },
         name: "ambassador-admin",
-        namespace: "kubeflow",
+        namespace: "istio-test",
       },
       spec: {
         ports: [
@@ -131,7 +133,7 @@ local testCases = [
       kind: "ServiceAccount",
       metadata: {
         name: "ambassador",
-        namespace: "kubeflow",
+        namespace: "istio-test",
       },
     },
   },
@@ -152,7 +154,7 @@ local testCases = [
         {
           kind: "ServiceAccount",
           name: "ambassador",
-          namespace: "kubeflow",
+          namespace: "istio-test",
         },
       ],
     },
@@ -165,7 +167,7 @@ local testCases = [
         kind: "Deployment",
         metadata: {
           name: "ambassador",
-          namespace: "kubeflow",
+          namespace: "istio-test",
         },
         spec: {
           replicas: 3,
@@ -174,7 +176,7 @@ local testCases = [
               labels: {
                 service: "ambassador",
               },
-              namespace: "kubeflow",
+              namespace: "istio-test",
             },
             spec: {
               containers: [

diff --git a/kubeflow/gcp/basic-auth-ingress.libsonnet b/kubeflow/gcp/basic-auth-ingress.libsonnet
@@ -4,9 +4,11 @@
   new(_env, _params):: {
     local params = _params + _env {
       hostname: if std.objectHas(_params, "hostname") then _params.hostname else "null",
-      ingressName: "envoy-ingress"
+      ingressName: "envoy-ingress",
+      portName: "ambassador",
+      injectIstio: util.toBool(_params.injectIstio),
     },
-    local namespace = params.namespace,
+    local namespace = if params.injectIstio then params.istioNamespace else params.namespace,
 
     // Test if the given hostname is in the form of: "NAME.endpoints.PROJECT.cloud.goog"
     local isCloudEndpoint(str) = {
@@ -99,7 +101,7 @@
               "name: whoami-mapping",
               "prefix: /whoami",
               "rewrite: /whoami",
-              "service: whoami-app." + namespace,
+              "service: whoami-app." + params.namespace,
             ]),
         },  //annotations
       },
@@ -222,6 +224,10 @@
                     name: "INGRESS_NAME",
                     value: params.ingressName,
                   },
+                  {
+                    name: "PORT_NAME",
+                    value: params.portName,
+                  },
                 ],
                 volumeMounts: [
                   {
@@ -364,7 +370,7 @@
           },
         ],
       },
-    },  // iapIngress
+    },  // ingress
     ingress:: ingress,
 
     local certificate = if params.privateGKECluster == "false" then (
@@ -430,6 +436,47 @@
     ),
     cloudEndpoint:: cloudEndpoint,
 
+    // No deployments. This is used for annotation that directs traffic to
+    // ISTIO ingress gateway.
+    local istioMappingSvc = {
+      apiVersion: "v1",
+      kind: "Service",
+      metadata: {
+        labels: {
+          app: "istioMappingSvc",
+        },
+        name: "istio-mapping-service",
+        namespace: namespace,
+        annotations: {
+          "getambassador.io/config":
+            std.join("\n", [
+              "---",
+              "apiVersion: ambassador/v0",
+              "kind:  Mapping",
+              "name: istio-mapping",
+              "prefix_regex: true",
+              "prefix: /(?!whoami|kflogin).*",
+              "rewrite: \"\"",
+              "service: istio-ingressgateway." + namespace,
+              "precedence: 1",
+            ]),
+        },  //annotations
+      },
+      spec: {
+        ports: [
+          {
+            port: 80,
+            targetPort: 8081,
+          },
+        ],
+        selector: {
+          app: "istioMappingSvc",
+        },
+        type: "ClusterIP",
+      },
+    },
+    istioMappingSvc:: istioMappingSvc,
+
     parts:: self,
     all:: [
       self.initServiceAccount,
@@ -444,7 +491,9 @@
       self.ingress,
       self.certificate,
       self.cloudEndpoint,
-    ],
+    ] + if params.injectIstio then [
+      self.istioMappingSvc,
+    ] else [],
 
     list(obj=self.all):: k.core.v1.list.new(obj,),
   },

diff --git a/kubeflow/gcp/iap.libsonnet b/kubeflow/gcp/iap.libsonnet
@@ -9,7 +9,8 @@
       envoyAdminPort: 8001,
       envoyStatsPort: 8025,
       injectIstio: util.toBool(_params.injectIstio),
-      ingressName: "envoy-ingress"
+      ingressName: "envoy-ingress",
+      portName: "http2",
     },
     local namespace = if params.injectIstio then params.istioNamespace else params.namespace,
 
@@ -316,6 +317,10 @@
                     name: "INGRESS_NAME",
                     value: params.ingressName,
                   },
+                  {
+                    name: "PORT_NAME",
+                    value: params.portName,
+                  },
                 ] + if params.injectIstio then [
                   {
                     name: "USE_ISTIO",
@@ -403,6 +408,10 @@
                     name: "GOOGLE_APPLICATION_CREDENTIALS",
                     value: "/var/run/secrets/sa/admin-gcp-sa.json",
                   },
+                  {
+                    name: "PORT_NAME",
+                    value: params.portName,
+                  },
                 ] + if params.injectIstio then [
                   {
                     name: "USE_ISTIO",

diff --git a/kubeflow/gcp/prototypes/basic-auth-ingress.jsonnet b/kubeflow/gcp/prototypes/basic-auth-ingress.jsonnet
@@ -9,6 +9,8 @@
 // @optionalParam issuer string letsencrypt-prod The cert-manager issuer name.
 // @optionalParam ingressSetupImage string gcr.io/kubeflow-images-public/ingress-setup:latest The image for setting up ingress.
 // @optionalParam privateGKECluster string false Is the k8s cluster a private GKE cluster
+// @optionalParam injectIstio string false The namespace where Istio is installed
+// @optionalParam istioNamespace string istio-system The namespace where Istio is installed
 
 local basicauth = import "kubeflow/gcp/basic-auth-ingress.libsonnet";
 local instance = basicauth.new(env, params);

diff --git a/kubeflow/gcp/setup_backend.sh b/kubeflow/gcp/setup_backend.sh
@@ -4,6 +4,7 @@
 [ -z ${NAMESPACE} ] && echo Error NAMESPACE must be set && exit 1
 [ -z ${SERVICE} ] && echo Error SERVICE must be set && exit 1
 [ -z ${INGRESS_NAME} ] && echo Error INGRESS_NAME must be set && exit 1
+[ -z ${PORT_NAME} ] && echo Error PORT_NAME must be set && exit 1
 
 PROJECT=$(curl -s -H "Metadata-Flavor: Google" http://metadata.google.internal/computeMetadata/v1/project/project-id)
 if [ -z ${PROJECT} ]; then
@@ -22,7 +23,7 @@ gcloud auth activate-service-account --key-file=${GOOGLE_APPLICATION_CREDENTIALS
 # Print out the config for debugging
 gcloud config list
 
-NODE_PORT=$(kubectl --namespace=${NAMESPACE} get svc ${SERVICE} -o jsonpath='{.spec.ports[?(@.name=="http2")].nodePort}')
+NODE_PORT=$(kubectl --namespace=${NAMESPACE} get svc ${SERVICE} -o jsonpath='{.spec.ports[?(@.name=="'${PORT_NAME}'")].nodePort}')
 echo "node port is ${NODE_PORT}"
 
 while [[ -z ${BACKEND_NAME} ]]; do

diff --git a/kubeflow/gcp/update_backend.sh b/kubeflow/gcp/update_backend.sh
@@ -5,6 +5,7 @@
 [ -z ${NAMESPACE} ] && echo Error NAMESPACE must be set && exit 1
 [ -z ${SERVICE} ] && echo Error SERVICE must be set && exit 1
 [ -z ${INGRESS_NAME} ] && echo Error INGRESS_NAME must be set && exit 1
+[ -z ${PORT_NAME} ] && echo Error PORT_NAME must be set && exit 1
 
 PROJECT=$(curl -s -H "Metadata-Flavor: Google" http://metadata.google.internal/computeMetadata/v1/project/project-id)
 if [ -z ${PROJECT} ]; then
@@ -15,7 +16,7 @@ fi
 # Activate the service account, allow 5 retries
 for i in {1..5}; do gcloud auth activate-service-account --key-file=${GOOGLE_APPLICATION_CREDENTIALS} && break || sleep 10; done
 
-NODE_PORT=$(kubectl --namespace=${NAMESPACE} get svc ${SERVICE} -o jsonpath='{.spec.ports[?(@.name=="http2")].nodePort}')
+NODE_PORT=$(kubectl --namespace=${NAMESPACE} get svc ${SERVICE} -o jsonpath='{.spec.ports[?(@.name=="'${PORT_NAME}'")].nodePort}')
 echo node port is ${NODE_PORT}
 
 while [[ -z ${BACKEND_NAME} ]]; do

diff --git a/prow_config.yaml b/prow_config.yaml
@@ -61,8 +61,8 @@ workflows:
       platform: gke
       gkeApiVersion: v1
       workflowName: kfctl-go
-      useBasicAuth: true  
-      useIstio: false
+      useBasicAuth: true
+      useIstio: true
   # Only run  kfctl presubmit test with basic auth if
   # files related to basic auth are modified.
   - app_dir: kubeflow/kubeflow/testing/workflows
@@ -82,7 +82,7 @@ workflows:
       gkeApiVersion: v1
       workflowName: kfctl-go
       useBasicAuth: true
-      useIstio: false
+      useIstio: true
   # kfctl test runs tests on gke.
   - app_dir: kubeflow/kubeflow/testing/workflows
     component: kfctl_test