Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 22 additions & 2 deletions pkg/connector/invitation.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,21 +346,27 @@ func (i *invitationResourceType) Delete(ctx context.Context, resourceId *v2.Reso
}

var (
isRemoved = false
resp *github.Response
isRemoved = false
wasCancelled = false
hadRealError = false
resp *github.Response
)

for _, org := range orgs {
resp, err = i.client.Organizations.CancelInvite(ctx, org, invitationID)
if err == nil {
isRemoved = true
wasCancelled = true
continue
}
if isNotFoundError(resp) {
// Invitation is already gone (expired or previously cancelled).
// Desired state is achieved, so treat as success.
isRemoved = true
continue
}
// A non-404 failure (e.g. 5xx) leaves this org's state undetermined.
hadRealError = true
}

if !isRemoved {
Expand All @@ -374,6 +380,20 @@ func (i *invitationResourceType) Delete(ctx context.Context, resourceId *v2.Reso

var annotations annotations.Annotations
annotations.WithRateLimiting(restApiRateLimit)
if !wasCancelled && !hadRealError {
// No active cancellation succeeded and no org returned a non-404
// error, yet the invitation is considered removed: the provider
// authoritatively reported it already absent (404) for a well-formed
// invitation id against a configured org. Emit the typed already-absent
// marker so a delete retried after a crash is classified as success
// rather than an ambiguous NotFound (SPEC-09a; baton-sdk#1033). An
// active cancellation is ordinary success and intentionally carries no
// marker; an unresolved non-404 error means absence was not confirmed,
// so the marker is withheld even though the pre-existing success return
// is preserved. A future baton-sdk bump can replace this direct
// construction with resource.ResourceDoesNotExistAnnotations().
annotations.Append(&v2.ResourceDoesNotExist{})
}
return annotations, nil
}

Expand Down
157 changes: 157 additions & 0 deletions pkg/connector/invitation_delete_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
package connector

import (
"context"
"net/http"
"strings"
"testing"

v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2"
"github.com/google/go-github/v69/github"
"github.com/migueleliasweb/go-github-mock/src/mock"
"github.com/stretchr/testify/require"
)

// TestInvitationDelete exercises invitationResourceType.Delete against the
// go-github-mock router (Pattern B: inline mock.NewMockedHTTPClient +
// github.NewClient). CancelInvite issues DELETE
// /orgs/{org}/invitations/{invitation_id}; getOrgs returns the configured
// orgs without any API call, so only the CancelInvite endpoint needs a
// handler. The already-absent marker (&v2.ResourceDoesNotExist{}) is asserted
// via annotations.Annotations.Contains.
func TestInvitationDelete(t *testing.T) {
ctx := context.Background()

newBuilder := func(httpClient *http.Client) *invitationResourceType {
gh := github.NewClient(httpClient)
return InvitationBuilder(InvitationBuilderParams{
client: gh,
orgCache: newOrgNameCache(gh),
orgs: []string{invitationTestOrgLogin},
})
}

invitationResID := func(resource string) *v2.ResourceId {
return &v2.ResourceId{
ResourceType: resourceTypeInvitation.Id,
Resource: resource,
}
}

t.Run("confirmed absence emits marker", func(t *testing.T) {
client := mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.DeleteOrgsInvitationsByOrgByInvitationId,
notFoundHandler(),
),
)

annos, err := newBuilder(client).Delete(ctx, invitationResID("5551212"))
require.NoError(t, err)
require.True(t, annos.Contains(&v2.ResourceDoesNotExist{}),
"a 404 with no active cancellation must emit the already-absent marker")
})

t.Run("ordinary success emits no marker", func(t *testing.T) {
client := mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.DeleteOrgsInvitationsByOrgByInvitationId,
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNoContent)
}),
),
)

annos, err := newBuilder(client).Delete(ctx, invitationResID("5551212"))
require.NoError(t, err)
require.False(t, annos.Contains(&v2.ResourceDoesNotExist{}),
"an active cancellation (204) is ordinary success and must carry no marker")
})

t.Run("other error stays error, no marker", func(t *testing.T) {
client := mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.DeleteOrgsInvitationsByOrgByInvitationId,
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusInternalServerError)
}),
),
)

annos, err := newBuilder(client).Delete(ctx, invitationResID("5551212"))
require.Error(t, err)
require.Nil(t, annos, "a 500 is a real failure: no annotations and no marker")
})

t.Run("exact org and invitation id addressing preserved", func(t *testing.T) {
var gotPath string
client := mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.DeleteOrgsInvitationsByOrgByInvitationId,
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
gotPath = r.URL.Path
w.WriteHeader(http.StatusNoContent)
}),
),
)

annos, err := newBuilder(client).Delete(ctx, invitationResID("5551212"))
require.NoError(t, err)
require.Equal(t, "/orgs/test-org-12/invitations/5551212", gotPath,
"CancelInvite must address org=test-org-12 and id=5551212")
require.False(t, annos.Contains(&v2.ResourceDoesNotExist{}))
})

t.Run("multi-org: 404 plus a non-404 error withholds the marker but keeps success", func(t *testing.T) {
// One configured org reports the invitation already absent (404) while
// another returns a transient 5xx (state undetermined). The pre-existing
// behavior returns success (isRemoved is set by the 404), but absence was
// not authoritatively confirmed everywhere, so no marker is emitted.
const secondOrg = "test-org-99"
client := mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.DeleteOrgsInvitationsByOrgByInvitationId,
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if strings.Contains(r.URL.Path, secondOrg) {
w.WriteHeader(http.StatusInternalServerError)
return
}
w.WriteHeader(http.StatusNotFound)
}),
),
)

gh := github.NewClient(client)
b := InvitationBuilder(InvitationBuilderParams{
client: gh,
orgCache: newOrgNameCache(gh),
orgs: []string{invitationTestOrgLogin, secondOrg},
})

annos, err := b.Delete(ctx, invitationResID("5551212"))
require.NoError(t, err)
require.False(t, annos.Contains(&v2.ResourceDoesNotExist{}),
"an unresolved non-404 error means absence was not confirmed: no marker")
})

t.Run("malformed invitation id errors before any API call", func(t *testing.T) {
// No CancelInvite handler: the parse failure must short-circuit before
// any HTTP request is issued.
client := mock.NewMockedHTTPClient()

annos, err := newBuilder(client).Delete(ctx, invitationResID("not-a-number"))
require.Error(t, err)
require.Nil(t, annos)
})

t.Run("wrong resource type errors", func(t *testing.T) {
client := mock.NewMockedHTTPClient()

annos, err := newBuilder(client).Delete(ctx, &v2.ResourceId{
ResourceType: resourceTypeUser.Id,
Resource: "5551212",
})
require.Error(t, err)
require.Nil(t, annos)
})
}
Loading