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

Split istio-external-auth into two components #2670

Merged
merged 1 commit into from
May 2, 2024

Conversation

codablock
Copy link
Contributor

Description of your changes:
First component contains only the patches. This is meant to be used from the istio deployment.

The second component contains the AuthorizationPolicy and RequestAuthentication, which must always be deployed when oauth2-proxy is deployed, so it doesn't make sense to pull these in from the istio oauth2-proxy overlay.

Checklist:

  • Unit tests pass:
    Make sure you have installed kustomize == 5.2.1+
    1. make generate-changed-only
    2. make test

@kromanow94
Copy link
Contributor

Hey @codablock , this is very reasonable. I deployed that in my environment with vcluster, run a few tests based on this gist and can confirm that from functionality perspective all is working correctly

https://gist.github.com/kromanow94/3a40e45f4f5777734e67ac3da683354e

$ export ISTIO_INGRESSGATEWAY_NAMESPACE=istio-system
$ export ISTIO_INGRESSGATEWAY_ENDPOINT=localhost:8080
$ export KF_PROFILE=kubeflow-user-example-com
$ export EXPERIMENT_NAME=m2m-experiment

$ nohup kubectl port-forward --namespace $ISTIO_INGRESSGATEWAY_NAMESPACE svc/istio-ingressgateway 8080:80 &

$ curl -XPOST "$ISTIO_INGRESSGATEWAY_ENDPOINT/pipeline/apis/v2beta1/experiments" \
  -H "Authorization: Bearer $(kubectl -n kubeflow-user-example-com create token default-editor)" \
  -d "{\"display_name\": \"$EXPERIMENT_NAME\", \"namespace\": \"$KF_PROFILE\"}" | jq
{
  "experiment_id": "950b5d75-382f-4b81-8da1-af58cf9c3878",
  "display_name": "m2m-experiment",
  "created_at": "2024-04-05T15:27:09Z",
  "namespace": "kubeflow-user-example-com",
  "storage_state": "AVAILABLE"
}

$ curl "$ISTIO_INGRESSGATEWAY_ENDPOINT/pipeline/apis/v2beta1/experiments?namespace=$KF_PROFILE" \
  -H "Authorization: Bearer $(kubectl -n kubeflow-user-example-com create token default-editor)" | jq
{
  "experiments": [
    {
      "experiment_id": "950b5d75-382f-4b81-8da1-af58cf9c3878",
      "display_name": "m2m-experiment",
      "created_at": "2024-04-05T15:27:09Z",
      "namespace": "kubeflow-user-example-com",
      "storage_state": "AVAILABLE"
    }
  ],
  "total_size": 1
}

$ curl "$ISTIO_INGRESSGATEWAY_ENDPOINT/pipeline/apis/v2beta1/pipelines" \
  -H "Authorization: Bearer $(kubectl -n kubeflow-user-example-com create token default-editor)" | jq
{
  "pipelines": [
    {
      "pipeline_id": "53f86345-3c2c-4c7a-8a90-6ba83c80b4d1",
      "display_name": "[Tutorial] Data passing in python components",
      "description": "[source code](https://github.com/kubeflow/pipelines/tree/c5658f09ec38e82730a8eca0a1aabf0876087eec/samples/tutorials/Data%20passing%20in%20python%20components) Shows how to pass data between python components.",
      "created_at": "2024-04-05T15:16:58Z"
    },
    {
      "pipeline_id": "a44f5b06-124f-46f0-8fbf-3e383ba4152b",
      "display_name": "[Tutorial] DSL - Control structures",
      "description": "[source code](https://github.com/kubeflow/pipelines/tree/c5658f09ec38e82730a8eca0a1aabf0876087eec/samples/tutorials/DSL%20-%20Control%20structures) Shows how to use conditional execution and exit handlers. This pipeline will randomly fail to demonstrate that the exit handler gets executed even in case of failure.",
      "created_at": "2024-04-05T15:16:59Z"
    }
  ],
  "total_size": 2
}

$ export EXPERIMENT_ID="$(curl -qs \
  "$ISTIO_INGRESSGATEWAY_ENDPOINT/pipeline/apis/v2beta1/experiments?namespace=$KF_PROFILE" \
  -H "Authorization: Bearer $(kubectl -n kubeflow-user-example-com create token default-editor)" \
  | jq --arg display_name $EXPERIMENT_NAME '.experiments[] | select(.display_name == $display_name)|.experiment_id' -r
  )"

$ export PIPELINE_ID="$(curl -qs \
  "$ISTIO_INGRESSGATEWAY_ENDPOINT/pipeline/apis/v2beta1/pipelines" \
  -H "Authorization: Bearer $(kubectl -n kubeflow-user-example-com create token default-editor)" \
  | jq '.pipelines[0].pipeline_id' -r
)"

$ curl -XPOST \
  "$ISTIO_INGRESSGATEWAY_ENDPOINT/pipeline/apis/v2beta1/runs?namespace=$KF_PROFILE" \
  -H "Authorization: Bearer $(kubectl -n kubeflow-user-example-com create token default-editor)" \
  -d "{
        \"experiment_id\":\"$EXPERIMENT_ID\",
        \"display_name\": \"Pipeline created with M2M Token\",
        \"pipeline_version_reference\": {\"pipeline_id\": \"$PIPELINE_ID\"}
    }" | jq
{
  "experiment_id": "950b5d75-382f-4b81-8da1-af58cf9c3878",
  "run_id": "d3706aed-0595-4802-a954-f92719f1ded2",
  "display_name": "Pipeline created with M2M Token",
  "storage_state": "AVAILABLE",
  "pipeline_version_reference": {
    "pipeline_id": "53f86345-3c2c-4c7a-8a90-6ba83c80b4d1",
    "pipeline_version_id": "e3eaf0c3-83a0-424e-ba3f-a92ee1ef1d91"
  },
  "service_account": "default-editor",
  "created_at": "2024-04-05T15:27:47Z",
  "scheduled_at": "2024-04-05T15:27:47Z",
  "finished_at": "1970-01-01T00:00:00Z",
  "state": "PENDING",
  "state_history": [
    {
      "update_time": "2024-04-05T15:27:47Z",
      "state": "PENDING"
    }
  ]
}

$ curl "$ISTIO_INGRESSGATEWAY_ENDPOINT/pipeline/apis/v2beta1/runs?namespace=$KF_PROFILE" \
  -H "Authorization: Bearer $(kubectl -n kubeflow-user-example-com create token default-editor)" | jq
{
  "runs": [
    {
      "experiment_id": "950b5d75-382f-4b81-8da1-af58cf9c3878",
      "run_id": "d3706aed-0595-4802-a954-f92719f1ded2",
      "display_name": "Pipeline created with M2M Token",
      "storage_state": "AVAILABLE",
      "pipeline_version_reference": {
        "pipeline_id": "53f86345-3c2c-4c7a-8a90-6ba83c80b4d1",
        "pipeline_version_id": "e3eaf0c3-83a0-424e-ba3f-a92ee1ef1d91"
      },
      "service_account": "default-editor",
      "created_at": "2024-04-05T15:27:47Z",
      "scheduled_at": "2024-04-05T15:27:47Z",
      "finished_at": "1970-01-01T00:00:00Z",
      "state": "RUNNING",
      "run_details": {
        "task_details": [
          {
            "run_id": "d3706aed-0595-4802-a954-f92719f1ded2",
            "task_id": "ab562944-eab1-4118-a505-5f6ab23c1bf9",
            "display_name": "tutorial-data-passing-6tvx2",
            "create_time": "2024-04-05T15:27:47Z",
            "start_time": "2024-04-05T15:27:47Z",
            "end_time": "0001-01-01T00:00:00Z",
            "state": "RUNNING",
            "state_history": [
              {
                "update_time": "2024-04-05T15:27:49Z",
                "state": "RUNNING"
              }
            ],
            "child_tasks": [
              {
                "pod_name": "tutorial-data-passing-6tvx2-3819678768"
              }
            ]
          },
          {
            "run_id": "d3706aed-0595-4802-a954-f92719f1ded2",
            "task_id": "62b4a391-92eb-41da-9672-39e4a4cfaf56",
            "display_name": "root-driver",
            "create_time": "2024-04-05T15:27:47Z",
            "start_time": "2024-04-05T15:27:47Z",
            "end_time": "0001-01-01T00:00:00Z",
            "state": "PENDING",
            "state_history": [
              {
                "update_time": "2024-04-05T15:27:49Z",
                "state": "PENDING"
              }
            ]
          }
        ]
      },
      "state_history": [
        {
          "update_time": "2024-04-05T15:27:47Z",
          "state": "PENDING"
        },
        {
          "update_time": "2024-04-05T15:27:48Z"
        },
        {
          "update_time": "2024-04-05T15:27:49Z",
          "state": "RUNNING"
        }
      ]
    }
  ],
  "total_size": 1
}

@juliusvonkohout
Copy link
Member

@codablock what about istio-cni?

@juliusvonkohout
Copy link
Member

/hold

First component contains only the patches. This is meant to be used from
the istio deployment.

The second component contains the AuthorizationPolicy and RequestAuthentication,
which must always be deployed when oauth2-proxy is deployed, so it doesn't
make sense to pull these in from the istio oauth2-proxy overlay.

Signed-off-by: Alexander Block <ablock84@gmail.com>
@codablock codablock force-pushed the fix-split-components branch from abf81e1 to 7d66918 Compare April 8, 2024 06:32
@codablock
Copy link
Contributor Author

@juliusvonkohout Indeed, forgot about that one, thanks. Fixed and force-pushed.

@juliusvonkohout
Copy link
Member

juliusvonkohout commented Apr 15, 2024

Sorry, our CICD is currently too slow for some tests.

@juliusvonkohout
Copy link
Member

/lgtm
/approve

Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: codablock, juliusvonkohout

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@juliusvonkohout
Copy link
Member

juliusvonkohout commented May 2, 2024

/unhold

the cicd seems to be healthy again

@google-oss-prow google-oss-prow bot merged commit 0aedaaf into kubeflow:master May 2, 2024
8 checks passed
@codablock codablock deleted the fix-split-components branch May 2, 2024 10:08
doncorsean pushed a commit to doncorsean/kubeflow-manifests that referenced this pull request Jul 18, 2024
First component contains only the patches. This is meant to be used from
the istio deployment.

The second component contains the AuthorizationPolicy and RequestAuthentication,
which must always be deployed when oauth2-proxy is deployed, so it doesn't
make sense to pull these in from the istio oauth2-proxy overlay.

Signed-off-by: Alexander Block <ablock84@gmail.com>
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.

3 participants