Skip to content

Update AccessToken Status When Site is Deleted #2416

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.

Sign up for GitHub

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

Open
shailie-1998 wants to merge 1 commit into
base: main
Choose a base branch
from
+301 −8
Open

Update AccessToken Status When Site is Deleted #2416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
80 changes: 80 additions & 0 deletions internal/kube/site/site.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1102,6 +1102,7 @@ func (s *Site) Deleted() {
slog.String("namespace", s.namespace),
slog.String("name", s.name))
s.bindings.cleanup()
s.updateAccessTokensStatus(s.namespace)
s.setBindingsConfiguredStatus(stderrors.New("No active site"))
s.profiles.Stop()
}
Expand Down Expand Up @@ -1151,6 +1152,57 @@ func (s *Site) updateSiteStatus() error {
return nil
}

func (s *Site) updateAccessTokensStatus(namespace string) {
// List all access tokens in the namespace
tokenList, err := s.clients.GetSkupperClient().SkupperV2alpha1().AccessTokens(namespace).List(context.TODO(), metav1.ListOptions{})
if err != nil {
s.logger.Error("Failed to list access tokens on site deletion",
slog.String("namespace", namespace),
slog.Any("error", err))
return
}

s.logger.Info("Updating access tokens for deleted site",
slog.String("namespace", namespace))

// Update status for each redeemed access token
for _, token := range tokenList.Items {
s.updateAccessTokenStatus(&token, "", "")
}
}

func (s *Site) updateAccessTokenStatus(token *skupperv2alpha1.AccessToken, remoteSiteId string, remoteSiteName string) {
// Only update if the token is redeemed
if !token.IsRedeemed() {
return
}

// Mark the token as failed due to site deletion
if token.SetRedeemedWithSiteDeletion(fmt.Errorf("Already redeemed for a deleted site")) {
_, err := s.clients.GetSkupperClient().SkupperV2alpha1().AccessTokens(token.Namespace).UpdateStatus(context.TODO(), token, metav1.UpdateOptions{})
if err != nil {
s.logger.Error("Failed to update access token status",
slog.String("namespace", token.Namespace),
slog.String("token", token.Name),
slog.String("remoteSiteId", remoteSiteId),
slog.String("remoteSiteName", remoteSiteName),
slog.Any("error", err))
} else {
if remoteSiteId != "" {
s.logger.Info("Updated access token status due to remote site removal",
slog.String("namespace", token.Namespace),
slog.String("token", token.Name),
slog.String("remoteSiteId", remoteSiteId),
slog.String("remoteSiteName", remoteSiteName))
} else {
s.logger.Info("Updated access token status due to site deletion",
slog.String("namespace", token.Namespace),
slog.String("token", token.Name))
}
}
}
}

func (s *Site) updateLinkOperationalCondition(link *skupperv2alpha1.Link, operational bool, remoteSiteId string, remoteSiteName string) error {
if link.SetOperational(operational, remoteSiteId, remoteSiteName) {
return s.updateLinkStatus(link)
Expand All @@ -1170,6 +1222,13 @@ func (s *Site) NetworkStatusUpdated(network []skupperv2alpha1.SiteRecord) error
if s.site == nil || reflect.DeepEqual(s.site.Status.Network, network) {
return nil
}

// Build a map of site IDs in the new network for quick lookup
activeSiteIds := make(map[string]bool)
for _, site := range network {
activeSiteIds[site.Id] = true
}

s.site.Status.Network = network
s.site.Status.SitesInNetwork = len(network)
updated, err := s.UpdateSiteStatus(s.site)
Expand All @@ -1190,6 +1249,27 @@ func (s *Site) NetworkStatusUpdated(network []skupperv2alpha1.SiteRecord) error
}
}
}

// Check all links to see if their remote sites are still in the network
// This catches links whose remote sites have been deleted (not in linkRecords)
for linkName, link := range s.links {
linkDef := link.Definition()
remoteSiteId := linkDef.Status.RemoteSiteId

// Update AccessToken status if the remote site is no longer in the network
if remoteSiteId != "" && !activeSiteIds[remoteSiteId] {
s.logger.Info("Detected link to deleted remote site, updating AccessToken",
slog.String("namespace", s.namespace),
slog.String("linkName", linkName),
slog.String("remoteSiteId", remoteSiteId))

// Get the AccessToken and update its status
token, err := s.clients.GetSkupperClient().SkupperV2alpha1().AccessTokens(s.namespace).Get(context.TODO(), linkName, metav1.GetOptions{})
if err == nil {
s.updateAccessTokenStatus(token, remoteSiteId, linkDef.Status.RemoteSiteName)
}
}
}
if config := s.bindings.networkUpdated(network); config != nil {
if err := s.updateRouterConfig(config); err != nil {
return err
Expand Down
212 changes: 204 additions & 8 deletions internal/kube/site/site_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -883,13 +883,15 @@ func Test_NetworkStatusUpdate(t *testing.T) {
siteRecord []skupperv2alpha1.SiteRecord
}
tests := []struct {
name string
args args
linkconfig *skupperv2alpha1.Link
wantErr bool
k8sObjects []runtime.Object
skupperObjects []runtime.Object
skupperErrorMessage string
name string
args args
linkconfig *skupperv2alpha1.Link
accessToken *skupperv2alpha1.AccessToken
wantErr bool
wantAccessTokenError bool
k8sObjects []runtime.Object
skupperObjects []runtime.Object
skupperErrorMessage string
}{
{
name: "no site",
Expand Down Expand Up @@ -945,11 +947,82 @@ func Test_NetworkStatusUpdate(t *testing.T) {
},
wantErr: false,
},
{
name: "remote site deleted - update access token",
args: args{
siteRecord: []skupperv2alpha1.SiteRecord{
{
Id: "local-site-id",
Name: "site-b",
Namespace: "test",
Platform: "kubernetes",
Version: "1.8.0",
// No links - remote site was deleted
},
},
},
linkconfig: &skupperv2alpha1.Link{
ObjectMeta: metav1.ObjectMeta{
Name: "site-a-token",
Namespace: "test",
UID: "link-uid",
},
Spec: skupperv2alpha1.LinkSpec{
Cost: 1,
Endpoints: []skupperv2alpha1.Endpoint{
{
Name: string(qdr.RoleInterRouter),
Host: "1.1.1.1",
Port: "55671",
},
},
},
Status: skupperv2alpha1.LinkStatus{
RemoteSiteId: "remote-site-id",
RemoteSiteName: "site-a",
},
},
accessToken: &skupperv2alpha1.AccessToken{
ObjectMeta: metav1.ObjectMeta{
Name: "site-a-token",
Namespace: "test",
Generation: 1,
},
Spec: skupperv2alpha1.AccessTokenSpec{
Url: "https://example.com",
Code: "test-code",
},
Status: skupperv2alpha1.AccessTokenStatus{
Redeemed: true,
Status: skupperv2alpha1.Status{
StatusType: "Ready",
Message: "OK",
Conditions: []metav1.Condition{
{
Type: "Redeemed",
Status: metav1.ConditionTrue,
Reason: "Ready",
Message: "OK",
LastTransitionTime: metav1.Now(),
ObservedGeneration: 1,
},
},
},
},
},
wantErr: false,
wantAccessTokenError: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s, err := newSiteMocks("test", tt.k8sObjects, tt.skupperObjects, tt.skupperErrorMessage, false)
skupperObjects := tt.skupperObjects
if tt.accessToken != nil {
skupperObjects = append(skupperObjects, tt.accessToken)
}

s, err := newSiteMocks("test", tt.k8sObjects, skupperObjects, tt.skupperErrorMessage, false)
assert.Assert(t, err)

// add link
Expand All @@ -969,10 +1042,31 @@ func Test_NetworkStatusUpdate(t *testing.T) {
if network.Platform == "podman" && network.Version == "1.8.0" {
foundConfig = true
}
if network.Platform == "kubernetes" && network.Version == "1.8.0" {
foundConfig = true
}
}
if foundConfig == false {
t.Errorf("Site.NetworkStatusUpdated() network not updated")
}

// verify access token was updated if remote site was deleted
if tt.wantAccessTokenError && tt.accessToken != nil {
token, err := s.clients.GetSkupperClient().SkupperV2alpha1().AccessTokens("test").Get(context.TODO(), tt.accessToken.Name, metav1.GetOptions{})
if err != nil {
t.Errorf("Site.NetworkStatusUpdated() failed to get access token: %v", err)
} else {
if token.Status.StatusType != "Error" {
t.Errorf("Site.NetworkStatusUpdated() access token status not updated, got StatusType=%s, want Error", token.Status.StatusType)
}
if token.Status.Message != "Already redeemed for a deleted site" {
t.Errorf("Site.NetworkStatusUpdated() access token message not updated, got Message=%s, want 'Already redeemed for a deleted site'", token.Status.Message)
}
if !token.Status.Redeemed {
t.Errorf("Site.NetworkStatusUpdated() access token should still be marked as redeemed")
}
}
}
}
})
}
Expand Down Expand Up @@ -1202,6 +1296,108 @@ func newSiteMocks(namespace string, k8sObjects []runtime.Object, skupperObjects
return newSite, nil
}

func TestSite_updateLinkOperationalCondition(t *testing.T) {
tests := []struct {
name string
link *skupperv2alpha1.Link
operational bool
remoteSiteId string
remoteSiteName string
}{
{
name: "Link becomes non-operational",
link: &skupperv2alpha1.Link{
ObjectMeta: metav1.ObjectMeta{
Name: "test-link",
Namespace: "test",
CreationTimestamp: metav1.Now(),
OwnerReferences: []metav1.OwnerReference{
{
Kind: "Site",
APIVersion: "skupper.io/v2alpha1",
Name: "site1",
UID: "8a96ffdf-403b-4e4a-83a8-97d3d459adb6",
},
},
},
Status: skupperv2alpha1.LinkStatus{
Status: skupperv2alpha1.Status{
Conditions: []metav1.Condition{
{
Type: skupperv2alpha1.CONDITION_TYPE_OPERATIONAL,
Status: metav1.ConditionTrue,
},
{
Type: skupperv2alpha1.CONDITION_TYPE_CONFIGURED,
Status: metav1.ConditionTrue,
},
},
},
},
},
operational: false,
remoteSiteId: "remote-site-id",
remoteSiteName: "remote-site",
},
{
name: "Link remains operational",
link: &skupperv2alpha1.Link{
ObjectMeta: metav1.ObjectMeta{
Name: "test-link",
Namespace: "test",
OwnerReferences: []metav1.OwnerReference{
{
Kind: "Site",
APIVersion: "skupper.io/v2alpha1",
Name: "site1",
UID: "8a96ffdf-403b-4e4a-83a8-97d3d459adb6",
},
},
},
Status: skupperv2alpha1.LinkStatus{
Status: skupperv2alpha1.Status{
Conditions: []metav1.Condition{
{
Type: skupperv2alpha1.CONDITION_TYPE_OPERATIONAL,
Status: metav1.ConditionTrue,
},
{
Type: skupperv2alpha1.CONDITION_TYPE_CONFIGURED,
Status: metav1.ConditionTrue,
},
},
},
},
},
operational: true,
remoteSiteId: "remote-site-id",
remoteSiteName: "remote-site",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s, err := newSiteMocks("test", nil, []runtime.Object{tt.link}, "", false)
assert.Assert(t, err)

// Initialize the link in the site's links map
if tt.link != nil {
linkObj := site1.NewLink(tt.link.Name, "")
linkObj.Update(tt.link)
s.links[tt.link.Name] = linkObj
}

// Call the function
err = s.updateLinkOperationalCondition(tt.link, tt.operational, tt.remoteSiteId, tt.remoteSiteName)
assert.Assert(t, err)

// Verify the link status was updated
assert.Equal(t, tt.link.Status.RemoteSiteId, tt.remoteSiteId)
assert.Equal(t, tt.link.Status.RemoteSiteName, tt.remoteSiteName)
})
}
}

func createRouterConfigMock(s *Site) error {
rc := qdr.InitialConfig(s.name+"-${HOSTNAME}", s.site.GetSiteId(), version.Version, s.isEdge(), 3)
rc.AddAddress(qdr.Address{
Expand Down
17 changes: 17 additions & 0 deletions pkg/apis/skupper/v2alpha1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,23 @@ func (t *AccessToken) SetRedeemed(err error) bool {
return false
}

func (t *AccessToken) SetRedeemedWithSiteDeletion(err error) bool {
// Set condition to True (redeemed) but with error message
// This prevents retry attempts while preserving the error information
state := ConditionState{
Status: v1.ConditionTrue, // Mark as True so IsRedeemed() returns true
Reason: "Error",
Message: err.Error(),
}
if t.Status.SetCondition(CONDITION_TYPE_REDEEMED, state, t.ObjectMeta.Generation) {
t.Status.Redeemed = true
t.Status.StatusType = state.Reason
t.Status.Message = state.Message
return true
}
return false
}

func (t *AccessToken) IsRedeemed() bool {
return meta.IsStatusConditionTrue(t.Status.Conditions, CONDITION_TYPE_REDEEMED)
}
Expand Down
Loading