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.

Sign up for GitHub

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

chore(dsl): Forbid global auth #16376

Closed
wants to merge 20 commits into from
Closed

chore(dsl): Forbid global auth #16376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
4f8db00
Typing for DSL
AndesKrrrrrrrrrrr Oct 11, 2024
12750ec
Force annotations
AndesKrrrrrrrrrrr Oct 11, 2024
d9d9330
No-as in web infra
AndesKrrrrrrrrrrr Oct 11, 2024
2883d1c
Only use "allowed" ingress specifications
AndesKrrrrrrrrrrr Oct 15, 2024
84017ce
Remove dead ingress
AndesKrrrrrrrrrrr Oct 15, 2024
109ee3d
Formatting
AndesKrrrrrrrrrrr Oct 15, 2024
abf0fcb
Don't require all attributes in ingress
AndesKrrrrrrrrrrr Oct 15, 2024
39066b5
Linting
AndesKrrrrrrrrrrr Oct 15, 2024
62f523c
Add test to verify global-auth override
AndesKrrrrrrrrrrr Oct 15, 2024
ef0637f
Add test to verify global-auth non-override
AndesKrrrrrrrrrrr Oct 15, 2024
fa13b0f
Conditional override
AndesKrrrrrrrrrrr Oct 15, 2024
5b62a7b
chore: charts update dirty files
andes-it Oct 15, 2024
1e1fbcf
Merge branch 'main' into chore/force-annotations
AndesKrrrrrrrrrrr Oct 15, 2024
6b38b06
chore: nx format:write update dirty files
andes-it Oct 15, 2024
1929ca3
Merge branch 'main' into chore/force-annotations
AndesKrrrrrrrrrrr Oct 15, 2024
43e7d90
Clearer intent for oveerrides (not "forced")
AndesKrrrrrrrrrrr Oct 16, 2024
0f0d5d8
Typing extraAnnotations
AndesKrrrrrrrrrrr Oct 16, 2024
dda828e
Adapt to configurable overrides by environment
AndesKrrrrrrrrrrr Oct 16, 2024
c55c693
Charts
AndesKrrrrrrrrrrr Oct 16, 2024
cba20de
Merge branch 'main' into chore/force-annotations
AndesKrrrrrrrrrrr Oct 16, 2024
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
6 changes: 3 additions & 3 deletions charts/identity-server/values.dev.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ identity-server:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/proxy-buffer-size: '8k'
nginx.ingress.kubernetes.io/proxy-buffering: 'on'
nginx.ingress.kubernetes.io/service-upstream: 'true'
Expand Down Expand Up @@ -267,7 +267,7 @@ services-auth-admin-api:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/service-upstream: 'true'
hosts:
- host: 'identity-server.dev01.devland.is'
Expand Down Expand Up @@ -782,7 +782,7 @@ services-auth-public-api:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/proxy-buffer-size: '8k'
nginx.ingress.kubernetes.io/proxy-buffering: 'on'
nginx.ingress.kubernetes.io/rewrite-target: '/$2'
Expand Down
6 changes: 3 additions & 3 deletions charts/identity-server/values.staging.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ identity-server:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/proxy-buffer-size: '8k'
nginx.ingress.kubernetes.io/proxy-buffering: 'on'
nginx.ingress.kubernetes.io/service-upstream: 'true'
Expand Down Expand Up @@ -267,7 +267,7 @@ services-auth-admin-api:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/service-upstream: 'true'
hosts:
- host: 'identity-server.staging01.devland.is'
Expand Down Expand Up @@ -782,7 +782,7 @@ services-auth-public-api:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/proxy-buffer-size: '8k'
nginx.ingress.kubernetes.io/proxy-buffering: 'on'
nginx.ingress.kubernetes.io/rewrite-target: '/$2'
Expand Down
12 changes: 6 additions & 6 deletions charts/islandis/values.dev.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ air-discount-scheme-backend:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/service-upstream: 'true'
hosts:
- host: 'loftbru.dev01.devland.is'
Expand Down Expand Up @@ -1079,7 +1079,7 @@ contentful-apps:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/service-upstream: 'true'
hosts:
- host: 'contentful-apps.dev01.devland.is'
Expand Down Expand Up @@ -1143,7 +1143,7 @@ contentful-entry-tagger-service:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/service-upstream: 'true'
hosts:
- host: 'contentful-entry-tagger-service.dev01.devland.is'
Expand Down Expand Up @@ -1481,7 +1481,7 @@ github-actions-cache:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/service-upstream: 'true'
hosts:
- host: 'cache.dev01.devland.is'
Expand Down Expand Up @@ -1647,7 +1647,7 @@ island-ui-storybook:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/service-upstream: 'true'
hosts:
- host: 'ui.dev01.devland.is'
Expand Down Expand Up @@ -1991,7 +1991,7 @@ search-indexer-service:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/service-upstream: 'true'
hosts:
- host: 'search-indexer-service.dev01.devland.is'
Expand Down
20 changes: 10 additions & 10 deletions charts/islandis/values.staging.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ air-discount-scheme-backend:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/service-upstream: 'true'
hosts:
- host: 'loftbru.staging01.devland.is'
Expand Down Expand Up @@ -441,7 +441,7 @@ api:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/service-upstream: 'true'
hosts:
- host: 'beta.staging01.devland.is'
Expand Down Expand Up @@ -949,7 +949,7 @@ application-system-form:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/proxy-buffer-size: '8k'
nginx.ingress.kubernetes.io/proxy-buffering: 'on'
nginx.ingress.kubernetes.io/service-upstream: 'true'
Expand Down Expand Up @@ -1017,7 +1017,7 @@ consultation-portal:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/proxy-buffer-size: '8k'
nginx.ingress.kubernetes.io/proxy-buffering: 'on'
nginx.ingress.kubernetes.io/service-upstream: 'true'
Expand Down Expand Up @@ -1108,7 +1108,7 @@ download-service:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/service-upstream: 'true'
hosts:
- host: 'api.staging01.devland.is'
Expand Down Expand Up @@ -1391,7 +1391,7 @@ island-ui-storybook:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/service-upstream: 'true'
hosts:
- host: 'ui.staging01.devland.is'
Expand Down Expand Up @@ -1464,7 +1464,7 @@ license-api:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-internal-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/service-upstream: 'true'
hosts:
- host: 'license-api-xrd.internal.staging01.devland.is'
Expand Down Expand Up @@ -1568,7 +1568,7 @@ portals-admin:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/proxy-buffer-size: '8k'
nginx.ingress.kubernetes.io/proxy-buffering: 'on'
nginx.ingress.kubernetes.io/service-upstream: 'true'
Expand Down Expand Up @@ -1733,7 +1733,7 @@ search-indexer-service:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/service-upstream: 'true'
hosts:
- host: 'search-indexer-service.staging01.devland.is'
Expand Down Expand Up @@ -1866,7 +1866,7 @@ service-portal:
primary-alb:
annotations:
kubernetes.io/ingress.class: 'nginx-external-alb'
nginx.ingress.kubernetes.io/enable-global-auth: 'false'
nginx.ingress.kubernetes.io/enable-global-auth: 'true'
nginx.ingress.kubernetes.io/proxy-buffer-size: '8k'
nginx.ingress.kubernetes.io/proxy-buffering: 'on'
nginx.ingress.kubernetes.io/service-upstream: 'true'
Expand Down
37 changes: 33 additions & 4 deletions infra/src/dsl/dsl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,21 @@ import {
XroadConfig,
PodDisruptionBudget,
IngressMapping,
OpsEnv,
} from './types/input-types'
import { logger } from '../logging'
import { COMMON_SECRETS } from './consts'

export const OVERRIDE_ANNOTATIONS: NonNullable<Ingress['extraAnnotations']> = {
dev: {
'nginx.ingress.kubernetes.io/enable-global-auth': 'true',
},
staging: {
'nginx.ingress.kubernetes.io/enable-global-auth': 'true',
},
prod: {},
}

/**
* Allows you to make some properties of a type optional.
*
Expand Down Expand Up @@ -66,7 +77,7 @@ export class ServiceBuilder<ServiceType extends string> {
grantNamespaces: [],
grantNamespacesEnabled: false,
secrets: COMMON_SECRETS,
ingress: {},
ingress: {} as IngressMapping, // Force type conformance for initialization
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid forced type casting when initializing ingress

Using as IngressMapping to force type conformance can conceal type-related issues. It's better to initialize ingress with a proper default value that aligns with the IngressMapping type to maintain type safety.

Consider initializing ingress without type casting:

ingress: {},

If IngressMapping requires a specific structure, provide a default value that matches its definition.

namespace: 'islandis',
serviceAccountEnabled: false,
securityContext: {
Expand Down Expand Up @@ -460,10 +471,28 @@ export class ServiceBuilder<ServiceType extends string> {

/**
* You can allow ingress traffic (traffic from the internet) to your service by creating an ingress controller. Mapped to an [Ingress](https://kubernetes.io/docs/concepts/services-networking/ingress/#what-is-ingress).
* @param ingress - Ingress parameters
* Some `extraAnnotations` are overridden. See `OVERRIDE_ANNOTATIONS` for the list of overrides.
* @param ingresses - Ingress parameters
*/
ingress(ingress: IngressMapping) {
this.serviceDef.ingress = ingress
ingress(ingresses: IngressMapping): this {
this.serviceDef.ingress = ingresses

if (!ingresses.primary) {
// Early return if no primary
return this
}
const ingressAnnotations = ingresses.primary.extraAnnotations ?? {}
for (const [env, annotations] of Object.entries(ingressAnnotations)) {
for (const annotation of Object.keys(annotations)) {
const overrides = OVERRIDE_ANNOTATIONS[env as OpsEnv] ?? {}
if (!(annotation in overrides)) {
// Leave non-overriddes as is
continue
}
// Apply overrides
annotations[annotation] = overrides[annotation]
}
AndesKrrrrrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
}
return this
}

Expand Down
64 changes: 64 additions & 0 deletions infra/src/dsl/ingress.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,4 +148,68 @@ describe('Ingress definitions', () => {
},
})
})

it('Global auth annotation is overridden', async () => {
const sut = service('api').ingress({
primary: {
public: false,
host: { dev: 'a', staging: 'a', prod: 'a' },
paths: ['/api'],
extraAnnotations: {
dev: {},
staging: {
'nginx.ingress.kubernetes.io/enable-global-auth': 'false',
},
prod: {},
},
},
})
const result = (await generateOutputOne({
outputFormat: renderers.helm,
service: sut,
runtime: new Kubernetes(Staging),
env: Staging,
})) as SerializeSuccess<HelmService>

expect(result.serviceDef[0].ingress).toEqual({
'primary-alb': {
annotations: {
'kubernetes.io/ingress.class': 'nginx-internal-alb',
'nginx.ingress.kubernetes.io/service-upstream': 'true',
'nginx.ingress.kubernetes.io/enable-global-auth': 'true',
},
hosts: [{ host: 'a.internal.staging01.devland.is', paths: ['/api'] }],
},
})
})
it('Global auth annotation not set if missing', async () => {
const sut = service('api').ingress({
primary: {
public: false,
host: { dev: 'a', staging: 'a', prod: 'a' },
paths: ['/api'],
extraAnnotations: {
dev: {},
staging: {},
prod: {},
},
},
})
const result = (await generateOutputOne({
outputFormat: renderers.helm,
service: sut,
runtime: new Kubernetes(Staging),
env: Staging,
})) as SerializeSuccess<HelmService>

expect(result.serviceDef[0].ingress).toEqual({
'primary-alb': {
annotations: {
'kubernetes.io/ingress.class': 'nginx-internal-alb',
'nginx.ingress.kubernetes.io/service-upstream': 'true',
},
hosts: [{ host: 'a.internal.staging01.devland.is', paths: ['/api'] }],
},
})
})
})
6 changes: 3 additions & 3 deletions infra/src/dsl/types/input-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,15 +133,15 @@ export type ServiceDefinitionForEnv = ServiceDefinitionCore & {
extraAttributes?: ExtraValuesForEnv
}

export type ExtraAnnotation = { [annotation: string]: string | null }
export type ExtraAnnotations = Partial<{ [env in OpsEnv]: ExtraAnnotation }>
export interface Ingress {
host: {
[env in OpsEnv]: string | string[]
}
paths: string[]
public?: boolean
extraAnnotations?: {
[env in OpsEnv]: { [annotation: string]: string | null }
}
extraAnnotations?: ExtraAnnotations
}

export interface IngressForEnv {
Expand Down
Loading