Migrate istio and dex to V3

[PatrickXYS]

Which issue is resolved by this Pull Request:
Resolves #1424

Description of your changes:

1. Migrate Istio, Istio-1-3-1, and Dex applications to V3.
2. Adopt corresponding changes for AWS.

Checklist:

• Unit tests have been rebuilt:
  1. cd manifests/tests
  2. make generate-changed-only
  3. make test

[kubeflow-bot]

This change is 

[PatrickXYS]

/cc @Jeffwan Can you help review AWS changes?
/cc @krishnadurai @yanniszark Can you help review Istio + Dex changes?

Btw, let me know if you think it's necessary to divide Istio+Dex migration and AWS changes into 2 PR.

[Jeffwan]

@PatrickXYS I would suggest to split AWS changes and common changes to separate PR. then it would be easier to track or cherry-pick in the future

[krishnadurai]

I agree with @Jeffwan's suggestion of splitting the PR into 2.

[krishnadurai]

Looks good to me, otherwise.

[PatrickXYS]

@krishnadurai I have removed AWS change and will send another PR after this PR merged.

Can you help review the PR?

[PatrickXYS]

Just for ping @krishnadurai

I did not change any code related to istio / dex components. Only change made here is removing AWS changes.

Would you minding take another look? And I only created base_v3 folder, this will be no impact on existing components/depencies.

[Jeffwan]

I think v3 migration is not clear for these kind of manifest. Technically, we can use overlay/application.yaml. The only thing missing is -params need to be migrated to -config. We do have some similar cases not migrated yet but I am not sure if we want to migrate it or not

[Jeffwan]

probably overlay/application.yaml is needed as well. I think besides param -> config, there's no need to migrate to v3 for this one. Can user use overlay/applications.yaml directly?

[PatrickXYS]

param -> config

Yes this is the main reason I want to migrate to V3.

Can user use overlay/applications.yaml directly?

Yes they can.

I'm okay with not migrating these components to V3. I think it might cause some incompatibility issues in the future. Since vendors (AWS/GCP) have moved to V3, and use config as the routine.

For example, https://github.com/kubeflow/manifests/blob/master/stacks/aws/application/istio/kustomization.yaml#L7-L10

if we don't migrate to V3, these configMap will be XX-parameters, what do you think?

- name: istio-parameters
  behavior: merge
  envs:
  - params.env

[Jeffwan]

This is same as istio/istio/base/kustomize?

[PatrickXYS]

Right, they are the same.

[krishnadurai]

/lgtm

[PatrickXYS]

@terrytangyuan @animeshsingh @Bobgy Can you take a look?

It's now lgtm but need to be approved for shipment.

[terrytangyuan]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: terrytangyuan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• dex-auth/OWNERS [terrytangyuan]
• istio/OWNERS [terrytangyuan]
• istio-1-3-1/OWNERS [terrytangyuan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[karlschriek]

@PatrickXYS was there ever a separate PR for the AWS part? I don't see it linked anywhere and as far as I can tell the official aws kfdef still installs 1.1.6.

Looking at the rolled-back changes in this PR, it looks like bumping to 1.3.1 only requires replacing

    - kustomizeConfig:
        repoRef:
          name: manifests
          path: stacks/aws/application/cluster-local-gateway
      name: cluster-local-gateway
    - kustomizeConfig:
        repoRef:
          name: manifests
          path: stacks/aws/application/istio
        name: istio

with

    - kustomizeConfig:
        repoRef:
          name: manifests
          path: istio/cluster-local-gateway/base_v3
      name: cluster-local-gateway
    - kustomizeConfig:
        repoRef:
          name: manifests
          path: stacks/aws/application/cognito-istio
      name: istio

Is that correct?

[karlschriek]

No, can't be right, not in 1.2.0 at least. stacks/aws/application/cognito-istio etc. don't exist.

As far as I can tell looking at the manifests, the change for 1.2.0 is to replace:

    - kustomizeConfig:
        repoRef:
          name: manifests
          path: stacks/aws/application/istio-stack
      name: istio-stack
    - kustomizeConfig:
        repoRef:
          name: manifests
          path: stacks/aws/application/cluster-local-gateway
      name: cluster-local-gateway

with

    - kustomizeConfig:
        repoRef:
          name: manifests
          path: stacks/aws/application/istio-1-3-1-stack
      name: istio-stack
    - kustomizeConfig:
        repoRef:
          name: manifests
          path: stacks/aws/application/cluster-local-gateway-1-3-1
      name: cluster-local-gateway