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

oauth2-proxy and Istio mesh support #2516

Closed
kromanow94 opened this issue Aug 30, 2023 · 8 comments · Fixed by #2544
Closed

oauth2-proxy and Istio mesh support #2516

kromanow94 opened this issue Aug 30, 2023 · 8 comments · Fixed by #2544

Comments

@kromanow94
Copy link
Contributor

Description

This is a follow up for #2409.

It was discussed that the oauth2-proxy integration with Istio should be managed with Istio Mesh Config instead of EnvoyFilter. The exact setup and reasoning was described it this and following comments: #2409 (comment).

Summary: it's better to use mesh config with envoyExtAuthzHttp extension provider instead of EnvoyFilter because it's better supported, it's described by Istio to be used in this way, provides the integration with oauth2-proxy for the whole istio cluster that configurable with AuthorizationPolicy and mitigates issues described in istio/istio#27790.

As mentioned in the #2409, I'll provide a PR for this feature.

@kimwnasptd @juliusvonkohout @axel7083

@axel7083
Copy link
Contributor

axel7083 commented Aug 30, 2023

Thanks for opening this follow up issue, if you need any help for testing or contributing don't hesitate !

@kromanow94
Copy link
Contributor Author

@axel7083 thanks, definitely will ask for some tests :)!

@kimwnasptd, @juliusvonkohout, I wonder about the repository structure. Can you help me here?

oauth2-proxy with meshConfig

Should this be created in an overlay alongside version with EnvoyFilter? Then the tree could be something like:

common/oidc-client/oauth2-proxy/
├── base
└── overlays
    ├── envoyfilter
    └── meshconfig

Istio with envoyExtAuthzHttp

Changes required in Istio to enable this setup are not too complicated. The easiest solution would be to provide an overlay. Considering @juliusvonkohout comment about #2455 and the need to have multiple istios for some time, I have two propositions:

Simple overlay

The benefit would be that one could simply point the bases in kustomization.yaml files to different version of istio (so one of common/istio-1-16, common/istio-1-17, common/istio-1-18

common/istio-oauth2-proxy/
├── cluster-local-gateway
│   └── base
│       └── kustomization.yaml
├── istio-crds
│   └── base
│       └── kustomization.yaml
├── istio-install
│   └── base
│       ├── kustomization.yaml
│       └── patches
│           ├── istio-configmap-disable-tracing-enable-oauth2-proxy.yaml
│           └── jwt-refresh-interval.yaml
├── istio-namespace
│   └── base
│       └── kustomization.yaml
└── kubeflow-istio-resources
    └── base
        ├── authorizationpolicy.oauth2-proxy-istio-ingressgateway.yaml
        ├── kustomization.yaml
        ├── params.env
        ├── params.yaml
        └── requestauthentication.dex-jwt.yaml

Full kustomize

This would follow the instructions in common/istio-1-17/README.md Upgrade Istio Manifests. Mind the <- new and <- modified inline comments.

This would result in a structure similar to:

common/istio-1-17
├── cluster-local-gateway
│   └── base
│       ├── cluster-local-gateway.yaml
│       ├── gateway-authorizationpolicy.yaml
│       ├── gateway.yaml
│       ├── kustomization.yaml
│       └── patches
│           └── remove-pdb.yaml
├── istio-crds
│   └── base
│       ├── crd.yaml
│       └── kustomization.yaml
├── istio-install
│   └── base
│       ├── deny_all_authorizationpolicy.yaml
│       ├── gateway_authorizationpolicy.yaml
│       ├── gateway.yaml
│       ├── install.yaml
│       ├── kustomization.yaml
│       ├── patches
│       │   ├── disable-debugging.yaml
│       │   ├── istio-configmap-disable-tracing-enable-oauth2-proxy.yaml  <- modified
│       │   ├── jwt-refresh-interval.yaml                                 <- new
│       │   ├── remove-pdb.yaml
│       │   └── service.yaml
│       └── x-forwarded-host.yaml
├── istio-namespace
│   └── base
│       ├── kustomization.yaml
│       └── namespace.yaml
├── kubeflow-istio-resources
│   └── base
│       ├── authorizationpolicy.oauth2-proxy-istio-ingressgateway.yaml  <- new
│       ├── cluster-roles.yaml
│       ├── kf-istio-resources.yaml
│       ├── kustomization.yaml
│       ├── params.env                                                  <- new
│       ├── params.yaml                                                 <- new
│       └── requestauthentication.dex-jwt.yaml                          <- new
├── profile-overlay.yaml                                            <- modified
├── profile.yaml
├── README.md
└── split-istio-packages

Let me know what you think and if you have other idea on the directory structure.

@juliusvonkohout
Copy link
Member

Is there a reason that you want to use overlays instead of components ? https://github.com/kubernetes-sigs/kustomize/blob/master/examples/components.md

@kromanow94
Copy link
Contributor Author

kromanow94 commented Aug 31, 2023

@juliusvonkohout I had to take a reading on kustomize components. Yeah, this seems like a good idea. But then how do you imagine the dir structure?

@kromanow94
Copy link
Contributor Author

@juliusvonkohout friendly reminder.

I imagine the dir structure for the kustomize could be something like this:

common/istio/components
└── oauth2-proxy
    ├── authorizationpolicy.oauth2-proxy-istio-ingressgateway.yaml
    ├── kustomization.yaml
    ├── params.env
    ├── params.yaml
    ├── patches
    │   ├── cm.enable-oauth2-proxy.yaml
    │   └── deployment.jwt-refresh-interval.yaml
    └── requestauthentication.dex-jwt.yaml

What do you think?

@juliusvonkohout
Copy link
Member

juliusvonkohout commented Sep 6, 2023

@kromanow94 I am on a business trip but I'll try to join the manifest meeting tomorrow. So far the structure looks fine. But I would maybe move it one level higher to the same level as oidc-authservice.

With the kustomize component we can then patch istio stuff from /common/oauth2 without strict inheritance and overlays. Probably when we remove oidc-authservice we can move some stuff directly to /common/istio-1-17/

But let's discuss this tomorrow.

@jbottum
Copy link

jbottum commented Sep 7, 2023

We might want to get Solo contacts involved as they are Istio SMEs, marino.wijay@solo.io is a contact and I believe he will be at the Kubeflow Summit.

@juliusvonkohout
Copy link
Member

I discussed an hour with @kromanow94 and this is what we came up with @kimwnasptd

common/oidc-client/
├── components         <- because this is directory with just the kustomize components 
│   ├── istio-oauth2-proxy
│   │   ├── authorizationpolicy.oauth2-proxy-istio-ingressgateway.yaml
│   │   ├── kustomization.yaml
│   │   ├── params.env
│   │   ├── params.yaml
│   │   ├── patches
│   │   │   ├── cm.enable-oauth2-proxy.yaml
│   │   │   └── deployment.jwt-refresh-interval.yaml
│   │   ├── README.md
│   │   └── requestauthentication.dex-jwt.yaml
│   ├── README.md
│   └── self-signed
│       ├── job.set-kubernetes-jwks-in-requestauthentication.yaml
│       ├── patch
│       │   └── deployment.oauth2-proxy.trust-kubernetes-certs.yaml
│       └── README.md
├── oauth2-proxy         <- already defined
├── oidc-authservice     <- already defined
└── README.md

We call the subfolder components, because we use kustomize components instead of overlays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants