Fix: Fold bundle service and token broker to operator image #540
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -99,26 +99,3 @@ jobs: | |
| go-version-file: operator/go.mod | ||
| - name: Build | ||
| run: make build | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| {{- if .Values.bundleService.enabled }} | ||
| apiVersion: apps/v1 | ||
| kind: Deployment | ||
| metadata: | ||
| labels: | ||
| {{- include "chart.labels" . | nindent 4 }} | ||
| app.kubernetes.io/component: bundle-service | ||
| name: bundle-service | ||
| namespace: {{ .Release.Namespace }} | ||
| spec: | ||
| replicas: {{ .Values.bundleService.replicas }} | ||
| selector: | ||
| matchLabels: | ||
| app: bundle-service | ||
| {{- include "chart.selectorLabels" . | nindent 6 }} | ||
| template: | ||
| metadata: | ||
| labels: | ||
| app: bundle-service | ||
| {{- include "chart.selectorLabels" . | nindent 8 }} | ||
| app.kubernetes.io/component: bundle-service | ||
| spec: | ||
| containers: | ||
| - name: bundle-service | ||
| # Same image as the manager unless explicitly overridden; ENTRYPOINT is | ||
| # /manager, so `command` is what selects this binary. | ||
| image: {{ .Values.bundleService.container.image.repository | default .Values.controllerManager.container.image.repository }}:{{ .Values.bundleService.container.image.tag | default .Values.controllerManager.container.image.tag }} | ||
| imagePullPolicy: {{ .Values.bundleService.container.image.pullPolicy | default .Values.controllerManager.container.image.pullPolicy | default "IfNotPresent" }} | ||
| command: | ||
| - {{ .Values.bundleService.container.cmd }} | ||
| ports: | ||
| - name: http | ||
| containerPort: 8080 | ||
| protocol: TCP | ||
| env: | ||
| # Scopes which global-scope AuthorizationPolicy CRs are honoured: | ||
| # the watcher ignores global CRs outside the service's own namespace. | ||
| - name: POD_NAMESPACE | ||
| valueFrom: | ||
| fieldRef: | ||
| fieldPath: metadata.namespace | ||
| {{- range $key, $value := .Values.bundleService.container.env }} | ||
| - name: {{ $key }} | ||
| value: {{ $value | quote }} | ||
| {{- end }} | ||
| livenessProbe: | ||
| {{- toYaml .Values.bundleService.container.livenessProbe | nindent 12 }} | ||
| readinessProbe: | ||
| {{- toYaml .Values.bundleService.container.readinessProbe | nindent 12 }} | ||
| resources: | ||
| {{- toYaml .Values.bundleService.container.resources | nindent 12 }} | ||
| securityContext: | ||
| {{- toYaml .Values.bundleService.container.securityContext | nindent 12 }} | ||
| securityContext: | ||
| {{- toYaml .Values.bundleService.securityContext | nindent 8 }} | ||
| serviceAccountName: {{ .Values.bundleService.serviceAccountName }} | ||
| {{- end -}} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| {{- if and .Values.rbac.enable .Values.bundleService.enabled }} | ||
| # The service watches AuthorizationPolicy CRs cluster-wide to compose bundles. | ||
| # list+watch only: it serves from an informer cache and never writes status. | ||
| apiVersion: rbac.authorization.k8s.io/v1 | ||
| kind: ClusterRole | ||
| metadata: | ||
| labels: | ||
| {{- include "chart.labels" . | nindent 4 }} | ||
| app.kubernetes.io/component: bundle-service | ||
| name: rossoctl-bundle-service | ||
| rules: | ||
| - apiGroups: | ||
| - agent.rossoctl.dev | ||
| resources: | ||
| - authorizationpolicies | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch | ||
| --- | ||
| apiVersion: rbac.authorization.k8s.io/v1 | ||
| kind: ClusterRoleBinding | ||
| metadata: | ||
| labels: | ||
| {{- include "chart.labels" . | nindent 4 }} | ||
| app.kubernetes.io/component: bundle-service | ||
| name: rossoctl-bundle-service | ||
| roleRef: | ||
| apiGroup: rbac.authorization.k8s.io | ||
| kind: ClusterRole | ||
| name: rossoctl-bundle-service | ||
| subjects: | ||
| - kind: ServiceAccount | ||
| name: {{ .Values.bundleService.serviceAccountName }} | ||
| namespace: {{ .Release.Namespace }} | ||
| {{- end -}} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| {{- if and .Values.rbac.enable .Values.bundleService.enabled }} | ||
| apiVersion: v1 | ||
| kind: ServiceAccount | ||
| metadata: | ||
| labels: | ||
| {{- include "chart.labels" . | nindent 4 }} | ||
| app.kubernetes.io/component: bundle-service | ||
| name: {{ .Values.bundleService.serviceAccountName }} | ||
| namespace: {{ .Release.Namespace }} | ||
| {{- end -}} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| {{- if .Values.tokenBroker.enabled }} | ||
| apiVersion: apps/v1 | ||
| kind: Deployment | ||
| metadata: | ||
| labels: | ||
| {{- include "chart.labels" . | nindent 4 }} | ||
| app.kubernetes.io/component: token-broker | ||
| name: token-broker | ||
| namespace: {{ .Release.Namespace }} | ||
| spec: | ||
| # Sessions and the token cache are held in memory, so this must stay at 1. | ||
| # Scaling out requires shared session state first. Enforced rather than merely | ||
| # documented: splitting sessions across pods fails at runtime, in the OAuth | ||
| # flow, well away from whoever set the replica count. | ||
| {{- if .Values.tokenBroker.replicas }} | ||
| {{- fail "tokenBroker.replicas is not supported: sessions and the token cache are in-memory, so the broker must run a single replica. Scaling out requires shared session state first." }} | ||
| {{- end }} | ||
| replicas: 1 | ||
| selector: | ||
| matchLabels: | ||
|
Contributor
There was a problem hiding this comment. nit: the comment is exactly the right thing to write here —
Since the constraint is real correctness rather than preference, consider making it enforced instead of documented, so a future {{- if .Values.tokenBroker.replicas }}
{{- fail "tokenBroker.replicas is not supported: sessions and the token cache are in-memory, so the broker must run a single replica. Scaling out requires shared session state first." }}
{{- end }}
replicas: 1That way the reasoning in the comment is also the thing that stops the mistake. Purely optional — the current form is already clearer than most.
Contributor
Author
There was a problem hiding this comment. Taken. The constraint is correctness rather than preference, so the fail guard is a better home for the reasoning than a comment. The guard keys off the value being set at all rather than its value, since 1 is the only legal setting — which |
||
| app: token-broker | ||
| {{- include "chart.selectorLabels" . | nindent 6 }} | ||
| template: | ||
| metadata: | ||
| labels: | ||
| app: token-broker | ||
| {{- include "chart.selectorLabels" . | nindent 8 }} | ||
| app.kubernetes.io/component: token-broker | ||
| annotations: | ||
| # The broker terminates its own OAuth flows; no mesh sidecar required. | ||
| sidecar.istio.io/inject: "false" | ||
| spec: | ||
| containers: | ||
| - name: token-broker | ||
| # Same image as the manager unless explicitly overridden; ENTRYPOINT is | ||
| # /manager, so `command` is what selects this binary. | ||
| image: {{ .Values.tokenBroker.container.image.repository | default .Values.controllerManager.container.image.repository }}:{{ .Values.tokenBroker.container.image.tag | default .Values.controllerManager.container.image.tag }} | ||
| imagePullPolicy: {{ .Values.tokenBroker.container.image.pullPolicy | default .Values.controllerManager.container.image.pullPolicy | default "IfNotPresent" }} | ||
| command: | ||
| - {{ .Values.tokenBroker.container.cmd }} | ||
| ports: | ||
| - name: http | ||
| containerPort: 8190 | ||
| protocol: TCP | ||
| env: | ||
| - name: TOKEN_BROKER_PORT | ||
| value: "8190" | ||
| - name: OAUTH_CLIENT_ID | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ .Values.tokenBroker.oauth.existingSecret }} | ||
| key: {{ .Values.tokenBroker.oauth.clientIdKey }} | ||
| - name: OAUTH_CLIENT_SECRET | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ .Values.tokenBroker.oauth.existingSecret }} | ||
| key: {{ .Values.tokenBroker.oauth.clientSecretKey }} | ||
| # Host here MUST match httpRoute.hostname. | ||
| - name: OAUTH_CALLBACK_URL | ||
| value: {{ .Values.tokenBroker.oauth.callbackUrl | quote }} | ||
| - name: ALLOWED_REDIRECT_HOSTS | ||
| value: {{ .Values.tokenBroker.oauth.allowedRedirectHosts | quote }} | ||
| - name: RESOURCE_CONFIG | ||
| value: {{ .Values.tokenBroker.resourceConfig | quote }} | ||
| {{- with .Values.tokenBroker.oauth.authorizationEndpoint }} | ||
| - name: OAUTH_AUTHORIZATION_ENDPOINT | ||
| value: {{ . | quote }} | ||
| {{- end }} | ||
| {{- with .Values.tokenBroker.oauth.tokenEndpoint }} | ||
| - name: OAUTH_TOKEN_ENDPOINT | ||
| value: {{ . | quote }} | ||
| {{- end }} | ||
| {{- with .Values.tokenBroker.oauth.scopesSupported }} | ||
| - name: OAUTH_SCOPES_SUPPORTED | ||
| value: {{ . | quote }} | ||
| {{- end }} | ||
| {{- with .Values.tokenBroker.jwt.jwksUrl }} | ||
| - name: JWT_JWKS_URL | ||
| value: {{ . | quote }} | ||
| {{- end }} | ||
| {{- with .Values.tokenBroker.jwt.issuer }} | ||
| - name: JWT_ISSUER | ||
| value: {{ . | quote }} | ||
| {{- end }} | ||
| {{- with .Values.tokenBroker.jwt.audience }} | ||
| - name: JWT_AUDIENCE | ||
| value: {{ . | quote }} | ||
| {{- end }} | ||
| {{- with .Values.tokenBroker.sessionTimeout }} | ||
| - name: TOKEN_BROKER_SESSION_TIMEOUT | ||
| value: {{ . | quote }} | ||
| {{- end }} | ||
| {{- with .Values.tokenBroker.maxSessionsPerUser }} | ||
| - name: TOKEN_BROKER_MAX_SESSIONS_PER_USER | ||
| value: {{ . | quote }} | ||
| {{- end }} | ||
| {{- with .Values.tokenBroker.tokenWaitTimeout }} | ||
| - name: TOKEN_BROKER_TOKEN_WAIT_TIMEOUT | ||
| value: {{ . | quote }} | ||
| {{- end }} | ||
| livenessProbe: | ||
| {{- toYaml .Values.tokenBroker.container.livenessProbe | nindent 12 }} | ||
| readinessProbe: | ||
| {{- toYaml .Values.tokenBroker.container.readinessProbe | nindent 12 }} | ||
| resources: | ||
| {{- toYaml .Values.tokenBroker.container.resources | nindent 12 }} | ||
| securityContext: | ||
| {{- toYaml .Values.tokenBroker.container.securityContext | nindent 12 }} | ||
| securityContext: | ||
| {{- toYaml .Values.tokenBroker.securityContext | nindent 8 }} | ||
| serviceAccountName: {{ .Values.tokenBroker.serviceAccountName }} | ||
| {{- end -}} | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Removing
test-token-brokeris correct — the module is gone, and the job'sworking-directory: token-brokerwould fail outright. But it is worth checking what replaced its coverage, because the deleted job's own comment states it existed so that #537 "cannot recur silently."After this PR:
make testandvetrungo test ./.../go vet ./..., so the mergedinternal/tokenbroker/...andpkg/oauthpackages are compiled and tested. An unresolvable dependency — exactly token-broker module fails to build: authlib pinned to pre-rename commit with old module path #537 — is still caught. Good.make build, however, isgo build -o bin/manager cmd/main.go(Makefile:146). It does not build./cmd/bundle-service/or./cmd/token-broker/.So the two new
mainpackages are compiled in CI only as a side effect of the Docker image build. A build break confined to one of those twomain.gofiles would pass Lint, Unit Tests and Build, and surface in the image build — recoverable, but a noisier signal than a failingbuildjob.Cheapest fix is to extend the
buildtarget to match the Dockerfile:That also keeps
make buildhonest about what the image now ships, which is the same single-source-of-truth argument the chart rendering inkind-reload-all.shmakes.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.
Confirmed and fixed. Makefile:146 was go build -o bin/manager cmd/main.go, and neither new main package appeared anywhere in the Makefile — so a break confined to either would pass Lint, Unit Tests and Build and only surface in the image build. That's the silent-failure mode the deleted test-token-broker job existed to prevent.
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.
build now compiles all three, matching the Dockerfile. Verified: produces bin/manager, bin/bundle-service, bin/token-broker.