refactor(kafka): use Nais API for topic access grants - #769
Merged
Conversation
Contributor
📝 Changelog previewBelow is a preview of the Changelog that will be added to the next release. Only commit messages that follow the Conventional Commits specification will be included in the Changelog. v5.46.2 - 2026-09-08Full Changelog: v5.46.1...v5.46.2 ⚙️ Miscellaneous Changes
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The regenerated TeamVulnerabilitySummaryFilter serializes an optional enum without omitempty, which can send an invalid empty enum value and break vulnerability-related GraphQL queries.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Refactors Kafka topic access granting to use the Nais API (GraphQL updateKafkaTopic) instead of updating KafkaTopic CRDs directly via Kubernetes/Aiven, aligning Kafka access management with the centralized API surface.
Changes:
- Add/consume GraphQL schema + generated client support for Kafka topic grants and the
updateKafkaTopicmutation. - Update
kafka grant-accesscommand to validate access flags and call the new Nais API-backed implementation. - Refresh GraphQL schema/client generation, including new vulnerability priority fields and issue types from the upstream API schema.
File summaries
| File | Description |
|---|---|
| schema.graphql | Updates upstream GraphQL schema, including Kafka topic grant types/mutation and additional vulnerability/issue schema changes. |
| internal/naisapi/gql/generated.go | Regenerates genqlient code to include new enums/types and GrantAccessToKafkaTopic mutation wiring. |
| internal/kafka/kafka.go | Adds Kafka grant helper that calls the generated GraphQL mutation via Nais API client. |
| internal/kafka/command/grant_access.go | Switches CLI command from Aiven/K8s implementation to Nais API call; adds access validation. |
| internal/kafka/command/flag/flag.go | Introduces a typed/validated access flag with autocomplete. |
| internal/issues/issues.go | Updates issue handling to the new urgent external-ingress vulnerability issue type. |
| internal/aiven/grant_access.go | Removes the previous K8s-based ACL update implementation. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
32401
to
+32403
| EnvironmentName string `json:"environmentName"` | ||
| // Input for filtering team vulnerability summaries. | ||
| Priority CVEPriority `json:"priority"` |
| case "read", "write", "readwrite": | ||
| return nil | ||
| default: | ||
| return naistrix.Errorf("invalid access level: %s", *a) |
| ) | ||
| return nil | ||
| if err := kafka.GrantAccessToKafkaTopic(ctx, topicName, grantAccessTopicFlags.Team, grantAccessTopicFlags.Environment, grant); err != nil { | ||
| return naistrix.Errorf("Unable to grant access: %v", err.Error()) |
christeredvartsen
force-pushed
the
kafka
branch
from
September 8, 2026 13:32
a26b585 to
5c3b683
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The generated TeamVulnerabilitySummaryFilter now includes an optional enum field without omitempty, which can serialize as an invalid empty GraphQL enum value and break existing vulnerability queries.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
internal/naisapi/gql/generated.go:32403
- TeamVulnerabilitySummaryFilter.Priority is optional in the schema, but the generated struct field lacks
omitempty. When callers set only EnvironmentName (e.g. internal/vulnerability/vulnerability.go), the JSON variables will includepriority:"", which is invalid for a GraphQL enum and can cause requests to fail.
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
Comment on lines
+37
to
+48
| func (a *KafkaTopicGrantAccess) Validate() error { | ||
| valid := []string{"read", "write", "readwrite"} | ||
| if a == nil { | ||
| return naistrix.Errorf("access level is required, must be one of: %s", strings.Join(valid, ", ")) | ||
| } | ||
|
|
||
| if !slices.Contains(valid, string(*a)) { | ||
| return naistrix.Errorf("invalid access level: %q, must be one of: %s", *a, strings.Join(valid, ", ")) | ||
| } | ||
|
|
||
| return nil | ||
| } |
Comment on lines
+28
to
45
| ValidateFunc: naistrix.ValidateFuncs( | ||
| validation.RequireTeamAndEnvironment(grantAccessTopicFlags), | ||
| func(context.Context, *naistrix.Arguments) error { | ||
| return grantAccessTopicFlags.Access.Validate() | ||
| }, | ||
| ), | ||
| RunFunc: func(ctx context.Context, args *naistrix.Arguments, out *naistrix.OutputWriter) error { | ||
| access := grantAccessTopicFlags.Access | ||
| namespace := grantAccessTopicFlags.Team | ||
| topicName := args.Get("topic") | ||
| username := kafkaApplicationName(args.Get("username")) | ||
|
|
||
| if err := aiven.ValidAclPermission(access); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| newAcl := nais_kafka.TopicACL{ | ||
| Team: namespace, | ||
| Application: username, | ||
| Access: access, | ||
| } | ||
| accessResult, err := aiven.GrantAccessToTopic(ctx, namespace, topicName, string(grantAccessTopicFlags.Environment), newAcl) | ||
| if err != nil { | ||
| return err | ||
| subject := kafkaApplicationName(args.Get("username")) | ||
| grant := gql.KafkaTopicGrantInput{ | ||
| Subject: subject, | ||
| TeamName: grantAccessTopicFlags.Team, | ||
| Access: gql.KafkaTopicGrantAccess(strings.ToUpper(string(grantAccessTopicFlags.Access))), | ||
| } | ||
|
|
||
| if accessResult.AlreadyAdded { | ||
| out.Printf( | ||
| "ACL entry already exists for '%s/%s' on topic %s/%s.", | ||
| newAcl.Application, newAcl.Access, namespace, topicName, | ||
| ) | ||
| return nil | ||
| if err := kafka.GrantAccessToKafkaTopic(ctx, topicName, grantAccessTopicFlags.Team, grantAccessTopicFlags.Environment, grant); err != nil { | ||
| return naistrix.Errorf("Unable to grant access: %s", err) | ||
| } |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.