Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions charts/controlplane/values.aws.selfhosted-intracluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -478,10 +478,22 @@ ingress:
- "{{ .Values.global.CONTROLPLANE_INTRA_CLUSTER_HOST }}"
secretName: "{{ .Values.global.TLS_SECRET_NAME }}"

# --- Ingress Annotations (shared across all ingress objects) ---
annotations:
# Allow the nginx controller's internal DNS to match ingress rules so that
# intra-cluster traffic (DP → CP via nginx service DNS) is routed through
# the same auth subrequest as external traffic. Without this, the :authority
# header won't match the ingress host and auth is bypassed.
nginx.ingress.kubernetes.io/server-alias: "{{ .Values.global.CONTROLPLANE_INTRA_CLUSTER_HOST }}"

# --- Protected Ingress Auth Annotations ---
# These configure nginx to validate requests via flyteadmin's /me endpoint
# and redirect unauthenticated users to /login for the OIDC flow.
# Active when OIDC authentication is enabled (server.security.useAuth: true).
#
# All protected endpoints use "https://$host/me" so the auth subrequest goes
# through nginx itself. This ensures verifyClaims runs on the access token,
# which resolves identitytype for all callers (browser, CLI, service-to-service).
protectedIngressAnnotations:
nginx.ingress.kubernetes.io/auth-url: "https://$host/me"
nginx.ingress.kubernetes.io/auth-signin: "https://$host/login?redirect_url=$escaped_request_uri"
Expand Down
9 changes: 8 additions & 1 deletion charts/controlplane/values.gcp.selfhosted-intracluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,14 @@ ingress:
- "{{ .Values.global.CONTROLPLANE_INTRA_CLUSTER_HOST }}"
secretName: "{{ .Values.global.TLS_SECRET_NAME }}"

# Protected ingress auth annotations are now defined in the base values.yaml.
# --- Ingress Annotations (shared across all ingress objects) ---
annotations:
# Allow the nginx controller's internal DNS to match ingress rules so that
# intra-cluster traffic (DP → CP via nginx service DNS) is routed through
# the same auth subrequest as external traffic.
nginx.ingress.kubernetes.io/server-alias: "{{ .Values.global.CONTROLPLANE_INTRA_CLUSTER_HOST }}"

# Protected ingress auth annotations are defined in the base values.yaml.
# Override here only if you need to customize auth behavior for this deployment mode.

# ----------------------------------------------------------------------------
Expand Down
57 changes: 57 additions & 0 deletions charts/controlplane/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,31 @@ ingress:
nginx.ingress.kubernetes.io/auth-url: "http://flyteadmin.{{ template \"flyte.namespace\" . }}.svc.cluster.local/me"
nginx.ingress.kubernetes.io/auth-response-headers: "Set-Cookie,X-User-Subject,X-User-Claim-Identitytype,X-User-Claim-Preferred-Username,X-User-Token"
nginx.ingress.kubernetes.io/auth-cache-key: "$http_authorization$http_flyte_authorization$http_cookie"
# For gRPC backends (backend-protocol: GRPC), nginx uses grpc_pass instead
# of proxy_pass. The auth-response-headers annotation only sets proxy headers,
# not gRPC headers. This configuration-snippet bridges identity headers from
# the auth subrequest response into the upstream gRPC request so backend
# services receive the caller's identity.
nginx.ingress.kubernetes.io/configuration-snippet: |
auth_request_set $user_id $upstream_http_x_user_subject;
proxy_set_header X-User-Subject $user_id;
grpc_set_header X-User-Subject $user_id;

Comment on lines +261 to +265
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The gRPC identity-forwarding configuration-snippet currently forwards X-User-Subject and X-User-Claim-Identitytype, but it does not forward X-User-Token or X-User-Claim-Preferred-Username even though they are included in auth-response-headers. If the intent is that gRPC backends receive the same identity context as HTTP backends, add grpc_set_header/proxy_set_header entries for these headers as well (sourced from the auth subrequest response).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these headers actually needed?

auth_request_set $user_identitytype $upstream_http_x_user_claim_identitytype;
proxy_set_header X-User-Claim-Identitytype $user_identitytype;
grpc_set_header X-User-Claim-Identitytype $user_identitytype;

auth_request_set $user_handle $upstream_http_x_user_claim_userhandle;
proxy_set_header X-User-Claim-userhandle $user_handle;
grpc_set_header X-User-Claim-userhandle $user_handle;

auth_request_set $groups $upstream_http_x_user_claim_groups;
proxy_set_header X-User-Claim-groups $groups;
grpc_set_header X-User-Claim-groups $groups;

Comment on lines +270 to +277
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The snippet reads $upstream_http_x_user_claim_userhandle and $upstream_http_x_user_claim_groups, but those headers are not included in nginx.ingress.kubernetes.io/auth-response-headers for this ingress. As written they will always be empty; either add the corresponding headers to auth-response-headers (if /me returns them) or drop these auth_request_set/proxy/grpc_set_header lines to avoid a misleading no-op.

Suggested change
auth_request_set $user_handle $upstream_http_x_user_claim_userhandle;
proxy_set_header X-User-Claim-userhandle $user_handle;
grpc_set_header X-User-Claim-userhandle $user_handle;
auth_request_set $groups $upstream_http_x_user_claim_groups;
proxy_set_header X-User-Claim-groups $groups;
grpc_set_header X-User-Claim-groups $groups;

Copilot uses AI. Check for mistakes.
more_set_headers "x-request-id: $request_id";
proxy_set_header x-request-id $request_id;
grpc_set_header x-request-id $request_id;

envoyGateway:
# GatewayClass name for Envoy Gateway. Used when INGRESS_PROVIDER is "envoy" or "both".
Expand Down Expand Up @@ -541,6 +566,38 @@ services:
secureTunnelTenantURLPattern: http://ingress-nginx-internal.ingress-nginx.svc.cluster.local:80 # http://ingress-nginx-internal.ingress-nginx.svc.cluster.local
clusterSelector:
type: local
organizations:
fullnameOverride: "organizations"
sharedService:
connectPort: 8081
initContainers:
Comment on lines +569 to +573
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR changes the rendered Helm output (new organizations service + new protectedIngressAnnotationsGrpc snippet). The repo has snapshot-style Helm output tests under tests/generated driven by tests/run.sh; these fixtures will need to be regenerated/updated or CI diffs will fail for the controlplane.* test cases.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude confirms this is why the CI check is failing.

- name: migrate
args:
- cloudorganizations
- migrate
- --config
- "/etc/config/*.yaml"
args:
- cloudorganizations
- serve
- --config
- /etc/config/*.yaml
configMap:
sharedService:
connectPort: 8081
metrics:
scope: "organizations:"
db:
dbname: '{{ .Values.global.DB_NAME }}'
host: '{{ .Values.global.DB_HOST }}'
username: '{{ .Values.global.DB_USER }}'
passwordPath: /etc/db/pass.txt
port: 5432
connectionPool:
maxIdleConnections: 20
maxOpenConnections: 20
maxConnectionLifetime: 1m

executions:
fullnameOverride: "executions"
initContainers:
Expand Down
Loading
Loading