From cc3bec1c0cfa0b09c8d9b64ae368fd3f48bad164 Mon Sep 17 00:00:00 2001 From: vmelikyan Date: Thu, 11 Jun 2026 22:27:55 -0700 Subject: [PATCH] fix: make environment service-account annotations cloud-neutral --- src/server/db/migrations/001_seed.ts | 2 +- .../__tests__/serviceAccountFactory.test.ts | 14 +- .../lib/agentSession/serviceAccountFactory.ts | 4 +- src/server/lib/kubernetes.ts | 97 +------- .../common/__tests__/serviceAccount.test.ts | 219 ++++++++++++++++++ .../lib/kubernetes/common/serviceAccount.ts | 97 +++++++- src/server/lib/kubernetes/rbac.ts | 101 +------- src/server/lib/nativeBuild/utils.ts | 9 - src/server/lib/nativeHelm/index.ts | 1 - src/server/lib/nativeHelm/utils.ts | 14 -- src/server/services/__tests__/build.test.ts | 5 +- src/server/services/build.ts | 6 +- src/server/services/types/globalConfig.ts | 5 +- 13 files changed, 335 insertions(+), 239 deletions(-) create mode 100644 src/server/lib/kubernetes/common/__tests__/serviceAccount.test.ts diff --git a/src/server/db/migrations/001_seed.ts b/src/server/db/migrations/001_seed.ts index 37a37bb6..692aa5c5 100644 --- a/src/server/db/migrations/001_seed.ts +++ b/src/server/db/migrations/001_seed.ts @@ -456,7 +456,7 @@ export async function up(knex: Knex): Promise { INSERT INTO global_config (key, config, "createdAt", "updatedAt", "deletedAt", description) VALUES ('rdsRestoreSettings', '{"vpcId":"","accountId":"","region":"us-west-2","securityGroupIds":[],"subnetGroupName":"","engine":"mysql","engineVersion":"8.0.33","tagMatch":{"key":"restore-for"},"instanceSize":"db.t3.small","restoreSize":"db.t3.small"}', now(), now(), null, 'Default RDS database settings to use for restore'); INSERT INTO global_config (key, config, "createdAt", "updatedAt", "deletedAt", description) VALUES ('minio', '{"version":"3.7.2","args":"--force --timeout 60m0s --wait","action":"install","chart":{"name":"minio","repoUrl":"https://charts.bitnami.com/bitnami","version":"15.0.7","values":[],"valueFiles":[]},"label":"labels","tolerations":"tolerations","affinity":"affinity","nodeSelector":"nodeSelector"}', now(), now(), null, 'Default minio s3 compatible bucket'); INSERT INTO global_config (key, config, "createdAt", "updatedAt", "deletedAt", description) VALUES ('features', '{"webhooks":true,"reconcileDeletedServices":false}', now(), now(), null, 'Configuration for feature flags controlled from database'); - INSERT INTO global_config (key, config, "createdAt", "updatedAt", "deletedAt", description) VALUES ('serviceAccount', '{"name": "default","role":"replace_me"}', now(), now(), null, 'Default IAM role name to be used to annotate service account'); + INSERT INTO global_config (key, config, "createdAt", "updatedAt", "deletedAt", description) VALUES ('serviceAccount', '{"name": "default"}', now(), now(), null, 'Service account for environment namespaces. Optional annotations map is applied to the generated service account (e.g. cloud workload identity keys).'); INSERT INTO global_config (key, config, "createdAt", "updatedAt", "deletedAt", description) VALUES ('app_setup', '{"state":"","created":false,"installed":false,"restarted":false,"org":"","url":"","name":""}', now(), now(), null, 'Application setup state'); INSERT INTO global_config (key, config, "createdAt", "updatedAt", "deletedAt", description) VALUES ('labels', '{"deploy":["lifecycle-deploy!"],"disabled":["lifecycle-disabled!"],"statusComments":["lifecycle-status-comments!"],"defaultStatusComments":true,"defaultControlComments":true}', now(), now(), null, 'Configurable PR labels for deploy, disabled, and status comments'); INSERT INTO global_config (key, config, "createdAt", "updatedAt", "deletedAt", description) VALUES ('logArchival', '{"enabled": false}', now(), now(), null, 'Log archival feature flag. Enable to archive build/deploy logs to object storage.'); diff --git a/src/server/lib/agentSession/__tests__/serviceAccountFactory.test.ts b/src/server/lib/agentSession/__tests__/serviceAccountFactory.test.ts index 2ecf2439..2358bfdc 100644 --- a/src/server/lib/agentSession/__tests__/serviceAccountFactory.test.ts +++ b/src/server/lib/agentSession/__tests__/serviceAccountFactory.test.ts @@ -14,10 +14,10 @@ * limitations under the License. */ -const mockSetupReadOnlyServiceAccountInNamespace = jest.fn(); +const mockEnsureServiceAccount = jest.fn(); -jest.mock('server/lib/kubernetes/rbac', () => ({ - setupReadOnlyServiceAccountInNamespace: mockSetupReadOnlyServiceAccountInNamespace, +jest.mock('server/lib/kubernetes/common/serviceAccount', () => ({ + ensureServiceAccount: mockEnsureServiceAccount, })); function loadModule() { @@ -36,7 +36,7 @@ describe('serviceAccountFactory', () => { it('reuses the in-flight setup promise for the same namespace', async () => { let resolveSetup!: () => void; - mockSetupReadOnlyServiceAccountInNamespace.mockImplementationOnce( + mockEnsureServiceAccount.mockImplementationOnce( () => new Promise((resolve) => { resolveSetup = resolve; @@ -47,7 +47,7 @@ describe('serviceAccountFactory', () => { const firstCall = ensureAgentSessionServiceAccount('test-ns'); const secondCall = ensureAgentSessionServiceAccount('test-ns'); - expect(mockSetupReadOnlyServiceAccountInNamespace).toHaveBeenCalledTimes(1); + expect(mockEnsureServiceAccount).toHaveBeenCalledTimes(1); resolveSetup(); @@ -57,12 +57,12 @@ describe('serviceAccountFactory', () => { it('clears the namespace cache after a failed setup', async () => { const setupError = new Error('setup failed'); - mockSetupReadOnlyServiceAccountInNamespace.mockRejectedValueOnce(setupError).mockResolvedValueOnce(undefined); + mockEnsureServiceAccount.mockRejectedValueOnce(setupError).mockResolvedValueOnce(undefined); const { ensureAgentSessionServiceAccount } = loadModule(); await expect(ensureAgentSessionServiceAccount('test-ns')).rejects.toThrow('setup failed'); await expect(ensureAgentSessionServiceAccount('test-ns')).resolves.toBe('agent-sa'); - expect(mockSetupReadOnlyServiceAccountInNamespace).toHaveBeenCalledTimes(2); + expect(mockEnsureServiceAccount).toHaveBeenCalledTimes(2); }); }); diff --git a/src/server/lib/agentSession/serviceAccountFactory.ts b/src/server/lib/agentSession/serviceAccountFactory.ts index ccc1840b..cb12ef78 100644 --- a/src/server/lib/agentSession/serviceAccountFactory.ts +++ b/src/server/lib/agentSession/serviceAccountFactory.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { setupReadOnlyServiceAccountInNamespace } from 'server/lib/kubernetes/rbac'; +import { ensureServiceAccount } from 'server/lib/kubernetes/common/serviceAccount'; export const AGENT_SESSION_SERVICE_ACCOUNT_NAME = 'agent-sa'; const serviceAccountSetupByNamespace = new Map>(); @@ -23,7 +23,7 @@ export async function ensureAgentSessionServiceAccount(namespace: string): Promi let setupPromise = serviceAccountSetupByNamespace.get(namespace); if (!setupPromise) { setupPromise = (async () => { - await setupReadOnlyServiceAccountInNamespace(namespace, AGENT_SESSION_SERVICE_ACCOUNT_NAME); + await ensureServiceAccount({ namespace, name: AGENT_SESSION_SERVICE_ACCOUNT_NAME, permissions: 'read' }); return AGENT_SESSION_SERVICE_ACCOUNT_NAME; })(); serviceAccountSetupByNamespace.set(namespace, setupPromise); diff --git a/src/server/lib/kubernetes.ts b/src/server/lib/kubernetes.ts index 11c9849f..a11be695 100644 --- a/src/server/lib/kubernetes.ts +++ b/src/server/lib/kubernetes.ts @@ -20,7 +20,7 @@ import _ from 'lodash'; import { Build, Deploy, Deployable, Service } from 'server/models'; import { CLIDeployTypes, KubernetesDeployTypes, MEDIUM_TYPE, DEFAULT_TTL_INACTIVITY_DAYS } from 'shared/constants'; import { shellPromise } from './shell'; -import { flattenObject, getKeepLabel, parsePullRequestLabels, waitUntil } from 'server/lib/utils'; +import { flattenObject, getKeepLabel, parsePullRequestLabels } from 'server/lib/utils'; import { ServiceDiskConfig } from 'server/models/yaml'; import * as k8s from '@kubernetes/client-node'; import { HttpError, V1Status, CoreV1Api, KubeConfig } from '@kubernetes/client-node'; @@ -28,7 +28,6 @@ import { IncomingMessage } from 'http'; import { APP_ENV, TMP_PATH } from 'shared/config'; import fs from 'fs'; import GlobalConfigService from 'server/services/globalConfig'; -import { setupServiceAccountWithRBAC } from './kubernetes/rbac'; import { staticEnvTolerations } from './helm/constants'; import { parseSecretRefsFromEnv, SecretRefWithEnvKey } from './secretRefs'; import { generateSecretName } from './kubernetes/externalSecret'; @@ -384,100 +383,6 @@ export async function createOrUpdateNamespace({ } } -export async function createOrUpdateServiceAccount({ namespace, role }: { namespace: string; role: string }) { - const kc = new k8s.KubeConfig(); - kc.loadFromDefault(); - const client = kc.makeApiClient(k8s.CoreV1Api); - - // Get the service account name from global config - const { serviceAccount } = await GlobalConfigService.getInstance().getAllConfigs(); - const serviceAccountName = serviceAccount?.name || 'default'; - - const serviceAccountExists = async () => { - try { - const saResponse = await client.readNamespacedServiceAccount(serviceAccountName, namespace); - return Boolean(saResponse?.body); - } catch (error) { - return false; - } - }; - - // If it's not the default service account, create it first - if (serviceAccountName !== 'default') { - const serviceAccountManifest = { - apiVersion: 'v1', - kind: 'ServiceAccount', - metadata: { - name: serviceAccountName, - }, - }; - - try { - if (!(await serviceAccountExists())) { - getLogger({ namespace, serviceAccountName }).debug('ServiceAccount: creating'); - await client.createNamespacedServiceAccount(namespace, serviceAccountManifest); - getLogger({ namespace, serviceAccountName }).debug('Created service account'); - } else { - getLogger({ namespace, serviceAccountName }).debug('Service account already exists'); - } - } catch (err) { - getLogger({ - namespace, - serviceAccountName, - error: err, - statusCode: err?.response?.statusCode, - statusMessage: err?.response?.statusMessage, - }).error('ServiceAccount: create failed'); - throw err; - } - } else { - try { - await waitUntil(serviceAccountExists, { - timeoutMs: 120000, - intervalMs: 2000, - }); - } catch (error) { - getLogger({ namespace, serviceAccountName, error }).error('ServiceAccount: wait timeout'); - throw error; - } - } - - // patch the service account with the role - const patch = { - metadata: { - annotations: { - 'eks.amazonaws.com/role-arn': role, - }, - }, - }; - - try { - await client.patchNamespacedServiceAccount( - serviceAccountName, - namespace, - patch, - undefined, - undefined, - undefined, - undefined, - undefined, - { headers: { 'Content-Type': 'application/merge-patch+json' } } - ); - getLogger({ namespace, serviceAccountName }).debug('Annotated service account'); - - await setupServiceAccountWithRBAC({ - namespace, - serviceAccountName, - awsRoleArn: role, - permissions: 'deploy', - }); - getLogger({ namespace, serviceAccountName }).debug('RBAC: configured'); - } catch (err) { - getLogger({ namespace, serviceAccountName, error: err }).error('ServiceAccount: setup failed'); - throw err; - } -} - /** * * @param build diff --git a/src/server/lib/kubernetes/common/__tests__/serviceAccount.test.ts b/src/server/lib/kubernetes/common/__tests__/serviceAccount.test.ts new file mode 100644 index 00000000..c549515a --- /dev/null +++ b/src/server/lib/kubernetes/common/__tests__/serviceAccount.test.ts @@ -0,0 +1,219 @@ +/** + * Copyright 2025 GoodRx, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const mockCreateNamespacedServiceAccount = jest.fn(); +const mockPatchNamespacedServiceAccount = jest.fn(); +const mockEnsureRoleAndBinding = jest.fn(); +const mockGetAllConfigs = jest.fn(); + +jest.mock('@kubernetes/client-node', () => ({ + KubeConfig: jest.fn().mockImplementation(() => ({ + loadFromDefault: jest.fn(), + makeApiClient: jest.fn(() => ({ + createNamespacedServiceAccount: (...args: unknown[]) => mockCreateNamespacedServiceAccount(...args), + patchNamespacedServiceAccount: (...args: unknown[]) => mockPatchNamespacedServiceAccount(...args), + })), + })), + CoreV1Api: jest.fn(), +})); + +jest.mock('../../rbac', () => ({ + ensureRoleAndBinding: (...args: unknown[]) => mockEnsureRoleAndBinding(...args), +})); + +jest.mock('server/services/globalConfig', () => ({ + __esModule: true, + default: { + getInstance: () => ({ getAllConfigs: (...args: unknown[]) => mockGetAllConfigs(...args) }), + }, +})); + +jest.mock('server/lib/logger', () => ({ + getLogger: () => ({ info: jest.fn(), warn: jest.fn(), error: jest.fn(), debug: jest.fn() }), +})); + +import { resolveServiceAccountAnnotations, ensureServiceAccount, ensureServiceAccountForJob } from '../serviceAccount'; + +const EKS_KEY = 'eks.amazonaws.com/role-arn'; +const GKE_KEY = 'iam.gke.io/gcp-service-account'; + +beforeEach(() => { + jest.clearAllMocks(); + mockCreateNamespacedServiceAccount.mockResolvedValue({}); + mockPatchNamespacedServiceAccount.mockResolvedValue({}); + mockEnsureRoleAndBinding.mockResolvedValue(undefined); +}); + +describe('resolveServiceAccountAnnotations', () => { + it('returns empty for missing config', () => { + expect(resolveServiceAccountAnnotations(undefined)).toEqual({}); + expect(resolveServiceAccountAnnotations({})).toEqual({}); + }); + + it('maps legacy role to the EKS annotation', () => { + expect(resolveServiceAccountAnnotations({ role: 'arn:aws:iam::123:role/lc' })).toEqual({ + [EKS_KEY]: 'arn:aws:iam::123:role/lc', + }); + }); + + it('trims the legacy role value', () => { + expect(resolveServiceAccountAnnotations({ role: ' arn:aws:iam::123:role/lc ' })).toEqual({ + [EKS_KEY]: 'arn:aws:iam::123:role/lc', + }); + }); + + it.each(['replace_me', 'default', '', ' '])('never emits placeholder role %j', (role) => { + expect(resolveServiceAccountAnnotations({ role })).toEqual({}); + }); + + it('passes explicit annotations through (GKE workload identity)', () => { + expect( + resolveServiceAccountAnnotations({ annotations: { [GKE_KEY]: 'sa@project.iam.gserviceaccount.com' } }) + ).toEqual({ [GKE_KEY]: 'sa@project.iam.gserviceaccount.com' }); + }); + + it('drops placeholder values inside explicit annotations', () => { + expect( + resolveServiceAccountAnnotations({ annotations: { [GKE_KEY]: 'replace_me', 'example.com/keep': 'yes' } }) + ).toEqual({ 'example.com/keep': 'yes' }); + }); + + it('lets explicit annotations win over the legacy role', () => { + expect( + resolveServiceAccountAnnotations({ + role: 'arn:aws:iam::123:role/old', + annotations: { [EKS_KEY]: 'arn:aws:iam::123:role/new' }, + }) + ).toEqual({ [EKS_KEY]: 'arn:aws:iam::123:role/new' }); + }); +}); + +describe('ensureServiceAccount', () => { + it('creates the service account with annotations and sets up RBAC', async () => { + await ensureServiceAccount({ + namespace: 'env-test', + name: 'lifecycle-sa', + annotations: { [GKE_KEY]: 'sa@project.iam.gserviceaccount.com' }, + permissions: 'deploy', + }); + + expect(mockCreateNamespacedServiceAccount).toHaveBeenCalledWith('env-test', { + metadata: { + name: 'lifecycle-sa', + namespace: 'env-test', + annotations: { [GKE_KEY]: 'sa@project.iam.gserviceaccount.com' }, + }, + }); + expect(mockPatchNamespacedServiceAccount).not.toHaveBeenCalled(); + expect(mockEnsureRoleAndBinding).toHaveBeenCalledWith({ + namespace: 'env-test', + serviceAccountName: 'lifecycle-sa', + permissions: 'deploy', + }); + }); + + it('patches when the service account already exists', async () => { + mockCreateNamespacedServiceAccount.mockRejectedValueOnce({ response: { statusCode: 409 } }); + + await ensureServiceAccount({ namespace: 'env-test', name: 'default', permissions: 'deploy' }); + + expect(mockPatchNamespacedServiceAccount).toHaveBeenCalledWith( + 'default', + 'env-test', + { metadata: { name: 'default', namespace: 'env-test', annotations: {} } }, + undefined, + undefined, + undefined, + undefined, + undefined, + { headers: { 'Content-Type': 'application/merge-patch+json' } } + ); + expect(mockEnsureRoleAndBinding).toHaveBeenCalled(); + }); + + it('rethrows non-conflict errors without touching RBAC', async () => { + mockCreateNamespacedServiceAccount.mockRejectedValueOnce({ response: { statusCode: 500 } }); + + await expect( + ensureServiceAccount({ namespace: 'env-test', name: 'default', permissions: 'deploy' }) + ).rejects.toEqual({ response: { statusCode: 500 } }); + expect(mockEnsureRoleAndBinding).not.toHaveBeenCalled(); + }); +}); + +describe('ensureServiceAccountForJob', () => { + it('ensures the named SA with resolved annotations plus the default SA', async () => { + mockGetAllConfigs.mockResolvedValue({ + serviceAccount: { name: 'lifecycle-sa', role: 'arn:aws:iam::123:role/lc' }, + }); + + const name = await ensureServiceAccountForJob('env-test', 'deploy'); + + expect(name).toBe('lifecycle-sa'); + expect(mockCreateNamespacedServiceAccount).toHaveBeenCalledTimes(2); + expect(mockCreateNamespacedServiceAccount).toHaveBeenNthCalledWith(1, 'env-test', { + metadata: { + name: 'lifecycle-sa', + namespace: 'env-test', + annotations: { [EKS_KEY]: 'arn:aws:iam::123:role/lc' }, + }, + }); + expect(mockCreateNamespacedServiceAccount).toHaveBeenNthCalledWith(2, 'env-test', { + metadata: { name: 'default', namespace: 'env-test', annotations: {} }, + }); + }); + + it('never emits the seeded replace_me placeholder', async () => { + mockGetAllConfigs.mockResolvedValue({ serviceAccount: { name: 'default', role: 'replace_me' } }); + + const name = await ensureServiceAccountForJob('env-test', 'webhook'); + + expect(name).toBe('default'); + expect(mockCreateNamespacedServiceAccount).toHaveBeenCalledTimes(1); + expect(mockCreateNamespacedServiceAccount).toHaveBeenCalledWith('env-test', { + metadata: { name: 'default', namespace: 'env-test', annotations: {} }, + }); + }); + + it('supports GKE workload identity via the annotations map', async () => { + mockGetAllConfigs.mockResolvedValue({ + serviceAccount: { + name: 'lifecycle-tools', + annotations: { [GKE_KEY]: 'lifecycle-tools@project.iam.gserviceaccount.com' }, + }, + }); + + await ensureServiceAccountForJob('env-test', 'build'); + + const saBody = mockCreateNamespacedServiceAccount.mock.calls[0][1]; + expect(saBody.metadata.annotations).toEqual({ + [GKE_KEY]: 'lifecycle-tools@project.iam.gserviceaccount.com', + }); + expect(saBody.metadata.annotations[EKS_KEY]).toBeUndefined(); + }); + + it('handles missing serviceAccount config', async () => { + mockGetAllConfigs.mockResolvedValue({}); + + const name = await ensureServiceAccountForJob('env-test', 'deploy'); + + expect(name).toBe('default'); + expect(mockCreateNamespacedServiceAccount).toHaveBeenCalledTimes(1); + expect(mockCreateNamespacedServiceAccount).toHaveBeenCalledWith('env-test', { + metadata: { name: 'default', namespace: 'env-test', annotations: {} }, + }); + }); +}); diff --git a/src/server/lib/kubernetes/common/serviceAccount.ts b/src/server/lib/kubernetes/common/serviceAccount.ts index 032099fd..ff35b491 100644 --- a/src/server/lib/kubernetes/common/serviceAccount.ts +++ b/src/server/lib/kubernetes/common/serviceAccount.ts @@ -14,23 +14,104 @@ * limitations under the License. */ +import * as k8s from '@kubernetes/client-node'; +import { V1ServiceAccount } from '@kubernetes/client-node'; import GlobalConfigService from 'server/services/globalConfig'; +import { RoleSettings } from 'server/services/types/globalConfig'; import { getLogger } from 'server/lib/logger'; -import { setupServiceAccountInNamespace } from '../../nativeHelm/utils'; +import { ensureRoleAndBinding, ServiceAccountPermissions } from '../rbac'; + +const PLACEHOLDER_VALUES = new Set(['', 'replace_me', 'default']); + +function isPlaceholder(value?: string): boolean { + return !value || PLACEHOLDER_VALUES.has(value.trim()); +} + +/** + * Renders global serviceAccount config into Kubernetes annotations. + * Legacy `role` maps to the EKS key; explicit `annotations` win on conflict. + * Placeholder values (empty, replace_me, default) are never emitted. + */ +export function resolveServiceAccountAnnotations( + settings?: Pick +): Record { + const annotations: Record = {}; + const role = settings?.role?.trim(); + if (role && !isPlaceholder(role)) { + annotations['eks.amazonaws.com/role-arn'] = role; + } + for (const [key, value] of Object.entries(settings?.annotations ?? {})) { + if (!isPlaceholder(value)) { + annotations[key] = value; + } + } + return annotations; +} + +/** + * Single reconciler for service accounts in environment namespaces: + * creates or patches the ServiceAccount with the given annotations, then + * ensures its namespace-scoped Role and RoleBinding. + */ +export async function ensureServiceAccount({ + namespace, + name, + annotations = {}, + permissions, +}: { + namespace: string; + name: string; + annotations?: Record; + permissions: ServiceAccountPermissions; +}): Promise { + const kc = new k8s.KubeConfig(); + kc.loadFromDefault(); + const coreV1Api = kc.makeApiClient(k8s.CoreV1Api); + + const serviceAccount: V1ServiceAccount = { + metadata: { name, namespace, annotations }, + }; + + try { + await coreV1Api.createNamespacedServiceAccount(namespace, serviceAccount); + getLogger().debug(`ServiceAccount: created ${name} namespace=${namespace}`); + } catch (error) { + if (error?.response?.statusCode === 409) { + await coreV1Api.patchNamespacedServiceAccount( + name, + namespace, + serviceAccount, + undefined, + undefined, + undefined, + undefined, + undefined, + { headers: { 'Content-Type': 'application/merge-patch+json' } } + ); + getLogger().debug(`ServiceAccount: updated ${name} namespace=${namespace}`); + } else { + getLogger({ namespace, serviceAccountName: name, error }).error('ServiceAccount: setup failed'); + throw error; + } + } + + await ensureRoleAndBinding({ namespace, serviceAccountName: name, permissions }); +} export async function ensureServiceAccountForJob( namespace: string, jobType: 'build' | 'deploy' | 'webhook' ): Promise { const { serviceAccount } = await GlobalConfigService.getInstance().getAllConfigs(); - const serviceAccountName = serviceAccount?.name || 'default'; - const role = serviceAccount?.role || 'default'; + const name = serviceAccount?.name || 'default'; + const annotations = resolveServiceAccountAnnotations(serviceAccount); - getLogger().info( - `ServiceAccount: setting up for job type=${jobType} namespace=${namespace} serviceAccount=${serviceAccountName} role=${role}` - ); + getLogger().info(`ServiceAccount: setting up for job type=${jobType} namespace=${namespace} serviceAccount=${name}`); - await setupServiceAccountInNamespace(namespace, serviceAccountName, role); + await ensureServiceAccount({ namespace, name, annotations, permissions: 'deploy' }); + if (name !== 'default') { + await ensureServiceAccount({ namespace, name: 'default', permissions: 'deploy' }); + } - return serviceAccountName; + return name; } diff --git a/src/server/lib/kubernetes/rbac.ts b/src/server/lib/kubernetes/rbac.ts index 207fd24e..314f975d 100644 --- a/src/server/lib/kubernetes/rbac.ts +++ b/src/server/lib/kubernetes/rbac.ts @@ -14,15 +14,16 @@ * limitations under the License. */ -import { V1ServiceAccount, V1Role, V1RoleBinding } from '@kubernetes/client-node'; +import { V1Role, V1RoleBinding } from '@kubernetes/client-node'; import * as k8s from '@kubernetes/client-node'; import { getLogger } from '../logger'; +export type ServiceAccountPermissions = 'build' | 'deploy' | 'full' | 'read'; + export interface RBACConfig { namespace: string; serviceAccountName: string; - awsRoleArn?: string; - permissions: 'build' | 'deploy' | 'full' | 'read'; + permissions: ServiceAccountPermissions; } const PERMISSION_RULES = { @@ -106,49 +107,13 @@ const PERMISSION_RULES = { ], }; -export async function setupServiceAccountWithRBAC(config: RBACConfig): Promise { - const { namespace, serviceAccountName, awsRoleArn, permissions } = config; +export async function ensureRoleAndBinding(config: RBACConfig): Promise { + const { namespace, serviceAccountName, permissions } = config; const kc = new k8s.KubeConfig(); kc.loadFromDefault(); - const coreV1Api = kc.makeApiClient(k8s.CoreV1Api); const rbacApi = kc.makeApiClient(k8s.RbacAuthorizationV1Api); - // Create or update ServiceAccount - const serviceAccount: V1ServiceAccount = { - metadata: { - name: serviceAccountName, - namespace, - annotations: awsRoleArn - ? { - 'eks.amazonaws.com/role-arn': awsRoleArn, - } - : {}, - }, - }; - - try { - await coreV1Api.createNamespacedServiceAccount(namespace, serviceAccount); - getLogger().debug(`ServiceAccount: created ${serviceAccountName} namespace=${namespace}`); - } catch (error) { - if (error?.response?.statusCode === 409) { - await coreV1Api.patchNamespacedServiceAccount( - serviceAccountName, - namespace, - serviceAccount, - undefined, - undefined, - undefined, - undefined, - undefined, - { headers: { 'Content-Type': 'application/merge-patch+json' } } - ); - getLogger().debug(`ServiceAccount: updated ${serviceAccountName} namespace=${namespace}`); - } else { - throw error; - } - } - // Create or update Role const roleName = `${serviceAccountName}-role`; const role: V1Role = { @@ -222,57 +187,3 @@ export async function setupServiceAccountWithRBAC(config: RBACConfig): Promise { - await setupServiceAccountWithRBAC({ - namespace, - serviceAccountName, - awsRoleArn, - permissions: 'build', - }); -} - -export async function setupReadOnlyServiceAccountInNamespace( - namespace: string, - serviceAccountName: string = 'agent-sa' -): Promise { - await setupServiceAccountWithRBAC({ - namespace, - serviceAccountName, - permissions: 'read', - }); -} - -export async function setupDeployServiceAccountInNamespace( - namespace: string, - serviceAccountName: string = 'default', - awsRoleArn?: string -): Promise { - await setupServiceAccountWithRBAC({ - namespace, - serviceAccountName, - awsRoleArn, - permissions: 'deploy', - }); - - if (serviceAccountName !== 'default') { - await setupServiceAccountWithRBAC({ - namespace, - serviceAccountName: 'default', - permissions: 'deploy', - }); - } -} - -export async function createServiceAccountUsingExistingFunction( - namespace: string, - _serviceAccountName: string, - role?: string -): Promise { - const { createOrUpdateServiceAccount } = await import('../kubernetes'); - await createOrUpdateServiceAccount({ namespace, role }); -} diff --git a/src/server/lib/nativeBuild/utils.ts b/src/server/lib/nativeBuild/utils.ts index 57acdf72..94edcb5a 100644 --- a/src/server/lib/nativeBuild/utils.ts +++ b/src/server/lib/nativeBuild/utils.ts @@ -17,17 +17,8 @@ import { V1Job } from '@kubernetes/client-node'; import GlobalConfigService from '../../services/globalConfig'; import { createBuildJob } from '../kubernetes/jobFactory'; -import { setupBuildServiceAccountInNamespace as setupServiceAccountWithRBAC } from '../kubernetes/rbac'; import { JobMonitor } from '../kubernetes/JobMonitor'; -export async function setupBuildServiceAccountInNamespace( - namespace: string, - serviceAccountName: string = 'native-build-sa', - awsRoleArn?: string -): Promise { - return setupServiceAccountWithRBAC(namespace, serviceAccountName, awsRoleArn); -} - export function createJob( name: string, namespace: string, diff --git a/src/server/lib/nativeHelm/index.ts b/src/server/lib/nativeHelm/index.ts index cca44db8..4f1efcdd 100644 --- a/src/server/lib/nativeHelm/index.ts +++ b/src/server/lib/nativeHelm/index.ts @@ -31,7 +31,6 @@ export { constructHelmCustomValues, constructHelmCommand, generateHelmInstallScript, - setupServiceAccountInNamespace, calculateJobTTL, createHelmJob, validateHelmConfiguration, diff --git a/src/server/lib/nativeHelm/utils.ts b/src/server/lib/nativeHelm/utils.ts index d5c19db0..75c658a9 100644 --- a/src/server/lib/nativeHelm/utils.ts +++ b/src/server/lib/nativeHelm/utils.ts @@ -29,10 +29,6 @@ import { scaffoldHelmSecretRefs, } from 'server/lib/helm/utils'; import { staticEnvTolerations } from 'server/lib/helm/constants'; -import { - createServiceAccountUsingExistingFunction, - setupDeployServiceAccountInNamespace, -} from 'server/lib/kubernetes/rbac'; import { getLogger } from 'server/lib/logger'; import { buildLifecycleLabels } from 'server/lib/kubernetes/labels'; import { normalizeKubernetesLabelValue } from 'server/lib/kubernetes/utils'; @@ -368,16 +364,6 @@ function mergeChartConfig(defaultChart: any, chartConfig: any, helmChart: any): }; } -export async function setupServiceAccountInNamespace( - namespace: string, - serviceAccountName: string, - role: string -): Promise { - await createServiceAccountUsingExistingFunction(namespace, serviceAccountName, role); - await setupDeployServiceAccountInNamespace(namespace, serviceAccountName, role); - getLogger().debug(`RBAC: configured serviceAccount=${serviceAccountName} namespace=${namespace}`); -} - export async function createNamespacedRoleAndBinding(namespace: string, serviceAccountName: string): Promise { const k8s = await import('@kubernetes/client-node'); const kc = new k8s.KubeConfig(); diff --git a/src/server/services/__tests__/build.test.ts b/src/server/services/__tests__/build.test.ts index 758ca7ae..cf8d3bde 100644 --- a/src/server/services/__tests__/build.test.ts +++ b/src/server/services/__tests__/build.test.ts @@ -87,7 +87,10 @@ jest.mock('server/lib/kubernetes', () => ({ applyManifests: (...args: any[]) => mockApplyManifests(...args), waitForPodReady: (...args: any[]) => mockWaitForPodReady(...args), createOrUpdateNamespace: jest.fn(), - createOrUpdateServiceAccount: jest.fn(), +})); + +jest.mock('server/lib/kubernetes/common/serviceAccount', () => ({ + ensureServiceAccountForJob: jest.fn().mockResolvedValue('default'), })); jest.mock('server/lib/github', () => ({ diff --git a/src/server/services/build.ts b/src/server/services/build.ts index c9851147..ecf43b53 100644 --- a/src/server/services/build.ts +++ b/src/server/services/build.ts @@ -39,6 +39,7 @@ import { ValidationError, YamlConfigValidator } from 'server/lib/yamlConfigValid import { type LifecycleYamlConfigOptions } from 'server/models/yaml/types'; import type { DeployableReconciliationResult } from 'server/services/deployable'; import { DeploymentManager } from 'server/lib/deploymentManager/deploymentManager'; +import { ensureServiceAccountForJob } from 'server/lib/kubernetes/common/serviceAccount'; import { Tracer } from 'server/lib/tracer'; import { redisClient } from 'server/lib/dependencies'; import { generateGraph } from 'server/lib/dependencyGraph'; @@ -1590,10 +1591,7 @@ export default class BuildService extends BaseService { staticEnv: build.isStatic, pullRequest: build.pullRequest, }); - await k8s.createOrUpdateServiceAccount({ - namespace: build.namespace, - role: serviceAccount?.role, - }); + await ensureServiceAccountForJob(build.namespace, 'deploy'); if (build.kind === BuildKind.ENVIRONMENT && build.uuid) { await new AgentPrewarmService(this.db, this.redis, this.redlock, this.queueManager) .queueBuildPrewarm(build.uuid) diff --git a/src/server/services/types/globalConfig.ts b/src/server/services/types/globalConfig.ts index 40822740..5cdd8237 100644 --- a/src/server/services/types/globalConfig.ts +++ b/src/server/services/types/globalConfig.ts @@ -172,8 +172,11 @@ export type SitesConfig = { }; export type RoleSettings = { - role: string; name?: string; + /** Annotations applied to service accounts in environment namespaces, e.g. cloud workload identity keys. */ + annotations?: Record; + /** @deprecated AWS-only: emitted as eks.amazonaws.com/role-arn. Use annotations instead. */ + role?: string; }; export type DatabaseSettings = {