-
Notifications
You must be signed in to change notification settings - Fork 1
Move Zalando Postgres stuff to own resolver, add some features #332
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
Conversation
This frees up the name Postgres for the general interface we want to add that should join the common fields of both the zalando postgres clusters and the cloud sql postgres databases. The current postgres mutations are not yet implemented by clients, so breaking them should be OK. Co-authored-by: Roger Bjørnstad <roger.bjornstad@nav.no> Co-authored-by: Morten Lied Johansen <morten.lied.johansen@nav.no>
Co-authored-by: Morten Lied Johansen <morten.lied.johansen@nav.no>
…ance and related functionality
…pplication, and Job types. No implementation for resolvers yet.
e86241b to
cfb8b0c
Compare
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.
Pull request overview
Refactors the existing “Postgres” (Zalando cluster) persistence/GraphQL implementation out of sqlinstance into a dedicated postgres package to free up the Postgres name for a future shared interface (Zalando + CloudSQL).
Changes:
- Moved Zalando Postgres watch/query/search/grant-access logic from
internal/persistence/sqlinstanceintointernal/persistence/postgres(nowPostgresInstance-based). - Updated GraphQL schema and resolvers to expose
PostgresInstancefields andgrantPostgresAccessvia the new package. - Wired up new watcher, dataloader context, search indexing, and updated integration test resources.
Reviewed changes
Copilot reviewed 43 out of 45 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/persistence/sqlinstance/search.go | Removes legacy Postgres search client registration from sqlinstance. |
| internal/persistence/sqlinstance/queries.go | Removes legacy Postgres getters and grant-access implementation from sqlinstance. |
| internal/persistence/sqlinstance/node.go | Removes legacy Postgres ident type registration from sqlinstance. |
| internal/persistence/sqlinstance/models.go | Removes legacy Postgres types/inputs; minor enum unmarshalling + SQLDatabase name tweak. |
| internal/persistence/sqlinstance/dataloader.go | Removes Postgres watcher plumbing from sqlinstance loader context. |
| internal/persistence/postgres/sortfilter.go | Adds sorting for PostgresInstance connections. |
| internal/persistence/postgres/search.go | Adds search client registration for Zalando Postgres instances. |
| internal/persistence/postgres/queries.go | Adds list/get/grant-access operations for Zalando Postgres instances. |
| internal/persistence/postgres/node.go | Adds ident type + parsing for PostgresInstance. |
| internal/persistence/postgres/models.go | Adds PostgresInstance model, order types, validation, and converter. |
| internal/persistence/postgres/dataloader.go | Adds Zalando Postgres watcher + loader context. |
| internal/persistence/postgres/activitylog.go | Moves Postgres grant-access activitylog transformer into postgres package. |
| internal/kubernetes/watchers/watchers.go | Renames watcher wiring from PostgresWatcher to ZalandoPostgresWatcher and points to new package. |
| internal/graph/sqlinstance.resolvers.go | Removes old Postgres mutation + Postgres resolvers from sqlinstance resolver file. |
| internal/graph/schema/sqlinstance.graphqls | Removes legacy Postgres GraphQL types/mutation/search extensions from sqlinstance schema file. |
| internal/graph/schema/postgres.graphqls | Introduces new PostgresInstance GraphQL schema + mutation/search extensions. |
| internal/graph/postgres.resolvers.go | Adds resolvers for PostgresInstance fields and grantPostgresAccess mutation. |
| internal/graph/gengql/valkey.generated.go | Regenerates to include new postgresInstances field references. |
| internal/graph/gengql/utilization.generated.go | Regenerates to include new postgresInstances field references. |
| internal/graph/gengql/teams.generated.go | Regenerates team/team-environment resolver interfaces + execution wiring for Postgres. |
| internal/graph/gengql/serviceaccounts.generated.go | Regenerates to include new postgresInstances field references. |
| internal/graph/gengql/secret.generated.go | Regenerates to include new postgresInstances field references. |
| internal/graph/gengql/search.generated.go | Regenerates SearchNode union handling for PostgresInstance. |
| internal/graph/gengql/schema.generated.go | Regenerates root schema wiring (mutation/input/node unions) for postgres package types. |
| internal/graph/gengql/root_.generated.go | Regenerates root interfaces + complexity scaffolding for new Postgres schema. |
| internal/graph/gengql/repository.generated.go | Regenerates to include new postgresInstances field references. |
| internal/graph/gengql/reconcilers.generated.go | Regenerates to include new postgresInstances field references. |
| internal/graph/gengql/postgres.generated.go | New gqlgen output for PostgresInstance schema/types. |
| internal/graph/gengql/persistence.generated.go | Regenerates Persistence union handling for PostgresInstance. |
| internal/graph/gengql/opensearch.generated.go | Regenerates to include new postgresInstances field references. |
| internal/graph/gengql/netpol.generated.go | Regenerates to include new postgresInstances field references. |
| internal/graph/gengql/kafka.generated.go | Regenerates to include new postgresInstances field references. |
| internal/graph/gengql/jobs.generated.go | Regenerates to include new postgresInstances field references. |
| internal/graph/gengql/issues.generated.go | Regenerates to include new postgresInstances field references. |
| internal/graph/gengql/complexity.go | Adds complexity calculation hook for Team.postgresInstances. |
| internal/graph/gengql/bucket.generated.go | Regenerates to include new postgresInstances field references. |
| internal/graph/gengql/bigquery.generated.go | Regenerates to include new postgresInstances field references. |
| internal/graph/gengql/applications.generated.go | Regenerates to include new postgresInstances field references. |
| internal/graph/gengql/alerts.generated.go | Regenerates to include new postgresInstances field references. |
| internal/graph/gengql/activitylog.generated.go | Regenerates ActivityLogEntry union handling for postgres activity log entries. |
| internal/cmd/api/http.go | Wires new watcher into search + adds postgres loader context middleware. |
| integration_tests/k8s_resources/grant_zalando_postgres_access/dev/someteamname/postgres_foobar.yaml | Adds Postgres CR test fixture for grant-access integration test. |
| integration_tests/grant_postgres_access.lua | Updates integration tests to use the new Zalando Postgres fixtures and GraphQL behavior. |
| .configs/gqlgen.yaml | Adds postgres package to gqlgen autobind list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func newZalandoPostgresIdent(teamSlug slug.Slug, environmentName, postgresInstanceName string) ident.Ident { | ||
| return ident.NewIdent(identZalandoPostgres, teamSlug.String(), environmentName, postgresInstanceName) | ||
| } |
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.
Hva er egentlig forskjell på newIdent og denne?
| var ( | ||
| specMajorVersion = []string{"spec", "cluster", "majorVersion"} | ||
| specCPU = []string{"spec", "cluster", "resources", "cpu"} | ||
| specMemory = []string{"spec", "cluster", "resources", "memory"} | ||
| specDiskSize = []string{"spec", "cluster", "resources", "diskSize"} | ||
| ) |
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.
Ser ut som at disse brukes ett sted i models.go, kan de ikke bare inlines eller flyttes dit?
| Version: "v1", | ||
| Resource: "rolebindings", | ||
| } | ||
| client, err := fromContext(ctx).zalandoPostgresWatcher.ImpersonatedClientWithNamespace(ctx, input.EnvironmentName, namespace, watcher.WithImpersonatedClientGVR(gvr)) |
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.
Har brukerne rettighet med sine egne brukere å opprette rollen og rollebindingen?
thokra-nav
left a comment
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.
Synes det ser fint ut. Uten at det er en blokker så hadde det vært fint med litt mer kommentarer/dokumentasjon i graphql fila så vi slepper å gjøre det senere med litt mindre husk om hva som er rett 😇
This PR moves Zalando Postgres stuff to it's own resolver and adds the ability to get information about Zalando Postgres clusters for workloads / teams.
Co-authored-by: Roger Bjørnstad roger.bjornstad@nav.no
Co-authored-by: Morten Lied Johansen morten.lied.johansen@nav.no