Add support for internal-frontend #602
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,6 +119,15 @@ data: | |
| membershipPort: {{ $server.frontend.service.membershipPort }} | ||
| bindOnIP: "0.0.0.0" | ||
|
|
||
| {{- if index $.Values.server "internal-frontend" "enabled" }} | ||
| internal-frontend: | ||
| rpc: | ||
| grpcPort: {{ index $server "internal-frontend" "service" "port" }} | ||
| httpPort: {{ index $server "internal-frontend" "service" "httpPort" }} | ||
| membershipPort: {{ index $server "internal-frontend" "service" "membershipPort" }} | ||
| bindOnIP: "0.0.0.0" | ||
| {{- end }} | ||
|
|
||
| history: | ||
| rpc: | ||
| grpcPort: {{ $server.history.service.port }} | ||
|
|
@@ -173,8 +182,10 @@ data: | |
| {{- toYaml . | nindent 6 }} | ||
| {{- end }} | ||
|
|
||
| {{- if not (index $.Values.server "internal-frontend" "enabled") }} | ||
|
There was a problem hiding this comment. Did you test it with authorization? When I was preparing similar changes I had to add condition to "authorization" + "publicClient" only for frontend. Otherwise internalFrontend will try to do auth and fail. Also, if we skip auth section for frontend then users will be able to access UI without permission. I ended up with creating a loop to make separated configmap per service {{- range $originalService := (list "frontend" "internalFrontend" "history" "matching" "worker") }}
{{ $serviceValues := index $.Values.server $originalService }}
{{ $service := include "serviceName" (list $originalService) }}
...
{{- if eq $service "frontend" }}
{{- with $server.config.authorization }}
authorization:
{{- toYaml . | nindent 10 }}
{{- end }}
{{- end }}
...
{{- if ne $service "frontend"}}
internal-frontend:
rpc:
grpcPort: {{ $server.internalFrontend.service.port }}
httpPort: {{ $server.internalFrontend.service.httpPort }}
membershipPort: {{ $server.internalFrontend.service.membershipPort }}
bindOnIP: "0.0.0.0"
{{- end }}There was a problem hiding this comment. @TheGeniesis We have tested it with authorization on frontend (in our case using OIDC) which is working, but I haven't specifically tested auth for internal-frontend. I was under the impression (possibly mistakenly?) that the purpose of internal-frontend was to bypass authorization for internal access, but perhaps there's a middle ground where it has a different level of auth configured. If you have insights or ideas here would appreciate any contributions to this PR as it's a bit outside of my wheelhouse. There was a problem hiding this comment. @TheGeniesis I don't think you're mistaken about the internal frontend purpose. It was introduced at temporalio/temporal#3706
In our use case we have https://docs.temporal.io/self-hosted-guide/security#plugins custom authorizer/claim mapper for JWT tokens issued by our auth server and there is no way other temporal components (e.g. server worker) could get/refresh the token required for frontend access, so internal-frontend solves exactly this case. There was a problem hiding this comment. I've successfully tested this PR with authorization enabled. Currently we're enabling auth using the built-in authorizer and claim plugins, and it's working without issue. I haven't leveraged the features that were failing when we didn't have the internal worker enabled (eg: scheduled jobs), but the system did come up without the errors and seemed to be working properly. I agree with @adamko147 about the internal frontend's purpose. There was a problem hiding this comment. I've been trying to find time in between meetings to test this, so I appreciate that confirmation, @dleblanc. There was a problem hiding this comment. Maybe I wasn't clear enough. About definitions Frontend should have authorization and should be used by users (direct calls/web). My auth scenario The EntraID was configured by following a medium article. The only difference is in system namespace which in the newer version is The problem which I faced The Frontend needs the "authorization" part and publicClient (since it's public). This MR How I solved the issue in my repo I see it's working for you. There might be one more factor which I'm not aware of and it was the reason why it didn't fully work for me. Unfortunately, I'm behind my working schedule and I don't have time to test deployments again with this MR, to confirm that I'm able to reproduce the same problem again :/ |
||
| publicClient: | ||
| hostPort: "{{ include "temporal.componentname" (list $ "frontend") }}:{{ $server.frontend.service.port }}" | ||
| {{- end }} | ||
|
|
||
| dynamicConfigClient: | ||
| filepath: "/etc/temporal/dynamic_config/dynamic_config.yaml" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -242,6 +242,33 @@ server: | |
| containerSecurityContext: {} | ||
| topologySpreadConstraints: [] | ||
| podDisruptionBudget: {} | ||
| internal-frontend: | ||
|
There was a problem hiding this comment. seems to me the helm chart is uses There was a problem hiding this comment. @adamko147 I had initially done this as There was a problem hiding this comment. Just as a proposition: you can create a simple helper to use internalFrontend as a "yaml" part and internal-frontend in names when needed: Usage: There was a problem hiding this comment. also, to be backwards compatible, should we disable internal frontend by default? There was a problem hiding this comment. @adamko147 agreed, that was meant to be disabled by default, I'll make this update |
||
| service: | ||
| # Evaluated as template | ||
| annotations: {} | ||
| type: ClusterIP | ||
| port: 7233 | ||
| membershipPort: 6933 | ||
| httpPort: 7243 | ||
|
There was a problem hiding this comment. Should these ports be 6936 and 7236 by default to match https://github.com/temporalio/temporal/blob/main/docker/config_template.yaml#L283-L284 There was a problem hiding this comment. Ports 6936 and 7236 also matches the suggestion in the release notes for version 1.20 (which introduced the internal-frontend). |
||
| metrics: | ||
| annotations: | ||
| enabled: true | ||
| serviceMonitor: {} | ||
| # enabled: false | ||
| prometheus: {} | ||
| # timerType: histogram | ||
| deploymentLabels: {} | ||
| deploymentAnnotations: {} | ||
| podAnnotations: {} | ||
| podLabels: {} | ||
| resources: {} | ||
| nodeSelector: {} | ||
| tolerations: [] | ||
| affinity: {} | ||
| additionalEnv: [] | ||
| containerSecurityContext: {} | ||
| topologySpreadConstraints: [] | ||
| podDisruptionBudget: {} | ||
| history: | ||
| service: | ||
| # type: ClusterIP | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamko147 Just a question about security:
If we use internal frontend here then everyone who has access to the admin-tools pod will be able to execute commands without any additional auth.
Is this expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, that’s how we use it in our use case. Temporal is running in dedicated cluster/namespace and only frontend is exposed to outside world. access to the k8s cluster controlled via IAM, so unless you don’t have cluster access, you don’t have admin tools access.
it’s also works better for ops person when troubleshooting, who’s logs into admin tools using kubectl and from there has access w/o need to generate access token. and as I said, kubectl access is controlled by IAM connected to our internal company IDP where we also set the auth access
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamko147 what you described matches our use case as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My internal "deny all" rule is screaming that it will be a problem for some people who has stricter polices related to this topic. But if code owners are ok with that then I'm ok too.