From f2eb5a2b36e07265e84f04b9fd5d3c4468334925 Mon Sep 17 00:00:00 2001 From: Vitalii Valkov Date: Tue, 1 Sep 2026 08:11:00 +0400 Subject: [PATCH] Add DeleteOrganizationResources Step 8 of the organization teardown: the org_nicknames the organization held. Last, because the identities those nicknames named are deleted by the services that registered them, in the steps above. The store method does not report an empty result as NotFound the way RemoveNickname does -- the step is retried, and finding nothing left is success, not a missing row. Internal RPC, so no permission check and no caller identity; the Istio policy settles that Organizations is the only caller. --- charts/identity/Chart.yaml | 4 +-- .../templates/authorization-policy.yaml | 31 ++++++++++++++++ charts/identity/values.yaml | 6 ++++ internal/server/nicknames.go | 19 ++++++++++ internal/server/nicknames_test.go | 36 +++++++++++++++++++ internal/server/server.go | 1 + internal/store/nicknames.go | 8 +++++ 7 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 charts/identity/templates/authorization-policy.yaml diff --git a/charts/identity/Chart.yaml b/charts/identity/Chart.yaml index e831581..e21d779 100644 --- a/charts/identity/Chart.yaml +++ b/charts/identity/Chart.yaml @@ -2,8 +2,8 @@ apiVersion: v2 name: identity description: Helm chart for deploying the IdentityService gRPC server. type: application -version: 0.1.0 -appVersion: 0.1.0 +version: 0.2.0 +appVersion: 0.2.0 home: https://github.com/agynio/identity sources: - https://github.com/agynio/identity diff --git a/charts/identity/templates/authorization-policy.yaml b/charts/identity/templates/authorization-policy.yaml new file mode 100644 index 0000000..eba7fd9 --- /dev/null +++ b/charts/identity/templates/authorization-policy.yaml @@ -0,0 +1,31 @@ +{{- if .Values.internalAuthorizationPolicy.enabled }} +{{- $namespace := .Release.Namespace }} +# DeleteOrganizationResources is one step of the organization teardown. It has +# no caller identity to authorize, so who may reach it at all is settled here. +apiVersion: security.istio.io/v1beta1 +kind: AuthorizationPolicy +metadata: + name: {{ .Release.Name }}-internal-identity + labels: + app.kubernetes.io/name: {{ include "service-base.name" . }} + app.kubernetes.io/instance: {{ .Release.Name }} +spec: + selector: + matchLabels: + app.kubernetes.io/name: {{ include "service-base.name" . }} + app.kubernetes.io/instance: {{ .Release.Name }} + action: DENY + rules: + - from: + - source: + notPrincipals: +{{- range .Values.internalAuthorizationPolicy.serviceAccounts }} + - "cluster.local/ns/{{ .namespace | default $namespace }}/sa/{{ .name }}" +{{- end }} + to: + - operation: + methods: + - POST + paths: + - /agynio.api.identity.v1.IdentityService/DeleteOrganizationResources +{{- end }} diff --git a/charts/identity/values.yaml b/charts/identity/values.yaml index 4532d40..fc53557 100644 --- a/charts/identity/values.yaml +++ b/charts/identity/values.yaml @@ -159,3 +159,9 @@ metrics: path: /metrics interval: "" scrapeTimeout: "" + +internalAuthorizationPolicy: + enabled: true + serviceAccounts: + - name: organizations + namespace: "" diff --git a/internal/server/nicknames.go b/internal/server/nicknames.go index 3e4d726..76da732 100644 --- a/internal/server/nicknames.go +++ b/internal/server/nicknames.go @@ -377,3 +377,22 @@ func (s *Server) authorizeNicknameRead(ctx context.Context, callerID uuid.UUID, } return nil } + +// DeleteOrganizationResources removes the organization's nicknames. It is +// internal: Istio settles who may call it, so there is no permission check and +// no caller identity to check against. Step 8 of the organization teardown, +// last because the identities that held these nicknames are deleted by the +// services that registered them, in the steps above. +// +// Idempotent: the delete is unconditional, so a retried step finds nothing to +// remove and still succeeds. +func (s *Server) DeleteOrganizationResources(ctx context.Context, req *identityv1.DeleteOrganizationResourcesRequest) (*identityv1.DeleteOrganizationResourcesResponse, error) { + organizationID, err := parseUUID(req.GetOrganizationId()) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "organization_id: %v", err) + } + if err := s.store.RemoveOrganizationNicknames(ctx, organizationID); err != nil { + return nil, status.Errorf(codes.Internal, "remove organization nicknames: %v", err) + } + return &identityv1.DeleteOrganizationResourcesResponse{}, nil +} diff --git a/internal/server/nicknames_test.go b/internal/server/nicknames_test.go index ba6127a..8128597 100644 --- a/internal/server/nicknames_test.go +++ b/internal/server/nicknames_test.go @@ -50,6 +50,8 @@ type fakeStore struct { lastInstanceSuffix *string resolution store.NicknameResolution registerErr error + removedOrgs []uuid.UUID + removeOrgErr error } func (f *fakeStore) RegisterIdentity(context.Context, uuid.UUID, int16) error { @@ -93,6 +95,14 @@ func (f *fakeStore) ResolveNickname(_ context.Context, _ uuid.UUID, nickname str return f.resolution, nil } +func (f *fakeStore) RemoveOrganizationNicknames(_ context.Context, organizationID uuid.UUID) error { + if f.removeOrgErr != nil { + return f.removeOrgErr + } + f.removedOrgs = append(f.removedOrgs, organizationID) + return nil +} + func (f *fakeStore) BatchGetNicknames(_ context.Context, organizationID uuid.UUID, identityIDs []uuid.UUID) (map[uuid.UUID]store.NicknameEntry, error) { f.lastOrganizationID = organizationID f.lastIdentityIDs = identityIDs @@ -306,3 +316,29 @@ func TestBatchGetNicknamesReturnsInstanceSuffix(t *testing.T) { require.Equal(t, "bob", response.Entries[0].GetNickname()) require.Equal(t, instanceSuffix, response.Entries[0].GetInstanceSuffix()) } + +func TestDeleteOrganizationResourcesRemovesNicknames(t *testing.T) { + organizationID := uuid.New() + fake := &fakeStore{} + server := New(fake, &fakeAuthClient{}) + + req := &identityv1.DeleteOrganizationResourcesRequest{OrganizationId: organizationID.String()} + // Internal RPC: no identity in the context, and none required. + _, err := server.DeleteOrganizationResources(context.Background(), req) + require.NoError(t, err) + + // The cascade retries a step it is unsure finished, so a second call has to + // succeed on the now-empty organization rather than report NotFound the way + // RemoveNickname would. + _, err = server.DeleteOrganizationResources(context.Background(), req) + require.NoError(t, err) + require.Equal(t, []uuid.UUID{organizationID, organizationID}, fake.removedOrgs) +} + +func TestDeleteOrganizationResourcesRejectsInvalidOrganizationID(t *testing.T) { + server := New(&fakeStore{}, &fakeAuthClient{}) + _, err := server.DeleteOrganizationResources(context.Background(), &identityv1.DeleteOrganizationResourcesRequest{ + OrganizationId: "not-a-uuid", + }) + require.Equal(t, codes.InvalidArgument, status.Code(err)) +} diff --git a/internal/server/server.go b/internal/server/server.go index 507b07b..ddc9675 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -22,6 +22,7 @@ type identityStore interface { RemoveNickname(context.Context, uuid.UUID, uuid.UUID, *uuid.UUID) error ResolveNickname(context.Context, uuid.UUID, string, *string) (store.NicknameResolution, error) BatchGetNicknames(context.Context, uuid.UUID, []uuid.UUID) (map[uuid.UUID]store.NicknameEntry, error) + RemoveOrganizationNicknames(context.Context, uuid.UUID) error } type authorizationChecker interface { diff --git a/internal/store/nicknames.go b/internal/store/nicknames.go index 42008cb..2025216 100644 --- a/internal/store/nicknames.go +++ b/internal/store/nicknames.go @@ -63,6 +63,14 @@ func (s *Store) RemoveNickname(ctx context.Context, organizationID uuid.UUID, id return nil } +// RemoveOrganizationNicknames drops every nickname held in the organization. +// Unlike RemoveNickname it does not report an empty result as NotFound: the +// teardown step it serves is retried, and finding nothing left is success. +func (s *Store) RemoveOrganizationNicknames(ctx context.Context, organizationID uuid.UUID) error { + _, err := s.pool.Exec(ctx, `DELETE FROM org_nicknames WHERE organization_id = $1`, organizationID) + return err +} + func (s *Store) ResolveNickname(ctx context.Context, organizationID uuid.UUID, nickname string, instanceSuffix *string) (NicknameResolution, error) { var resolution NicknameResolution var installationID pgtype.UUID