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.

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

SPArator - CRD for SPA #235

Open
Kyrremann opened this issue Oct 21, 2024 · 8 comments
Open

SPArator - CRD for SPA #235

Kyrremann opened this issue Oct 21, 2024 · 8 comments

Comments

@Kyrremann
Copy link
Contributor

Beskrivelse / Essensen

I dag tilbyr vi deploy av SPA via en Github Action, for de få som bruker denne løsningen så funker det bra. Problemet er at den er hardkodet til NAV, og at det vil være tungvindt å skrive den om til å støtte andre tenants.

Det action gjør er å først opprette to k8s-ressurser på runneren (ingress og service), så laster den opp assets med cdn-upload, før den bruker nais/deploy for å deploye de to ressursene den opprettet.
For å opprette ingress-ressursen trenger den å vite hvilke adresser en tenant støtter, og hva ingressClassen her. Den har heller ikke støtte for root-domener da vi ikke ønsker at noen ved et uhell skal ta over nav.no. Dette skaper problemer for nais.io og leesah.io.

Hvis vi flytter SPA-biten ut til en egen enkel CRD, så kan vi la hver enkel operator avgjøre hvilke domener som er godkjent, og hvilke ingressClass de skal ha. Det blir også enklere å vedlikeholde en liste med ekstra domener. Ved å ha SPArator i clusteret vil vi automatisk ha validering på om et domene+path er tatt i bruk, som vil være en sikring av tjenesten vår.

Etterpå kan vi da fjerne den gamle actionen, og la brukerne bruke cdn-upload som vanlig, og nais/deploy med den nye crd-filen.

Investeringsvilje

2 uker

Mulig løsning i grove trekk

  • Lage en operator (SPArator) som har støtte for en enkel CRD som tar i mot et domene.
    • SPArator må validere domene mot ingresser som finnes i clusteret, og finne ut hvilken ingressClass den skal bruke selv.
  • Dokumentere bruk av ny CRD
  • Støtte for konfigurasjon av ekstradomener via Fasit

Ikke-mål

  • Slette SPA (holder med en annonsering, så kan man lage en reminder for noen uker senere)

Eventuell annen relevant informasjon

Shapers

  • Kyrre Havik
@Reasonable-Solutions
Copy link

I like this initiative because I do not believe that using github actions as an interface is a good idea, less github actions, less problem.
@jhrv is this an archetype, so to speak?

Hvis vi flytter SPA-biten ut til en egen enkel CRD,

This is actually any content, like an MPA, that needs a URL, so there's maybe a distinction that can be made in the naming here.
otoh, Bucket-content-urlrator doesn't really roll of the tongue.

@Kyrremann
Copy link
Contributor Author

Godt poeng! Er nok mange dokumentasjonssider som kunne hatt nytte av dette, så de slapp å lage sine egne apper.

@jhrv
Copy link
Contributor

jhrv commented Oct 21, 2024

Syns dette er en god idé, og jeg sitter igjen med noen spørsmål:

  • Tenker vi fortsette med nginx og ingress + service, eller kunne man brukt Google CDN direkte? Kan fremdeles være en CRD for å holde på tilstanden
  • Gir dette mening å presentere som en tredje workload type, ved siden av apps og job? Eller er det bare CDN med en URL? Er det noen meningsfulle forskjeller på det vi kaller SPA (men som kanskje bør hete noe annet?) og bare vanlig CDN?
  • Ref punkt over, synes det er fint at vi med dette kan vise teamene tydeligere hva de har "kjørende" av ting og tang.
  • Det er noe med sammenhengen av miljøene og URLene som er litt utydelig og lite intuitiv i dag som kanskje kan/bør adresseres. F.eks om du vil knytte et område i CDN til URL med domene X, må du velge miljø Y. Kunne vi gjort det smartere? Eller må vi bare tilby en mapping-tabell de kan slå opp i?

Bucket-content-urlrator doesn't really roll of the tongue.

Helt uenig, kjempebra navn

@Kyrremann
Copy link
Contributor Author

  1. Vi kan bruke CDN direkte (ved å legge til host/path rules), usikker på hva vi faktisk sparer på å blande oss inn i den enkle syncen som Nais Console gjør for å rulle ut CDNer. Ved å ha ingressene i clusteret får vi som nevnt automatisk sjekk av like ingresser, selv om kanskje eventen er litt skjult.
  2. Høres ut som en god idé å få det inn som en ny workload, eller i det minste synliggjøre mulighetene man har med dette. For å gjøre det ryddig så tror jeg at det å skille mellom CDN og det andre (SPA) vil gjøre det enklere for brukerne våre. CDN er der du lagrer statiske filer, mens det andre er en egendefinert ingress. Og nå som jeg skriver det så kan det hende at alt vi trenger er en workloads > how to > Custom ingress for CDN.
  3. enig!
  4. Dette blir vel det samme som når du velger et domene for en "vanlig" app, vi har tabellen i dokumentasjonen som sier hvilke domener er tilgjengelig i hvilket cluster. Eneste forskjellen er at vi har kun et miljø for CDN, og lar de selv bestemme path.

Hva med å stjele navnet til Github pages, Nais pages?
Geocities -> Naiscities?
Googles sites -> Nais sites?

@jhrv
Copy link
Contributor

jhrv commented Oct 22, 2024

Dette blir vel det samme som når du velger et domene for en "vanlig" app

Godt poeng. Er akkurat like rart :)

Liker Nais pages.

Men sliter litt med distinksjonen/konseptet, for om du laster opp en statisk webside uten spa-deploy i dag, og peker på den URLen du havner på så får du jo opp sida like fint. Så blir ikke da Nais pages akkurat det samme, bare med en litt finere URL?

@Kyrremann
Copy link
Contributor Author

Men sliter litt med distinksjonen/konseptet, for om du laster opp en statisk webside uten spa-deploy i dag, og peker på den URLen du havner på så får du jo opp sida like fint. Så blir ikke da Nais pages akkurat det samme, bare med en litt finere URL?

Er egentlig bare det som er distinksjonen, med Nais pages kan du få en egendefinert domene.

@jhrv
Copy link
Contributor

jhrv commented Oct 23, 2024

jepp, så tenker derfor man kan argumentere for at dette bare er NAIS CDN sånn konseptuelt, men at vi gjør det mulig med en kjekkere URL.

@Kyrremann
Copy link
Contributor Author

PS: For å få nåværende oppsett til å fungere med test-nais har jeg manuelt lagt inn alle tenants og adresser.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants