refactor(filesystem): clean legacy digest entries on tag deletion
- Add digest filtering to removeFromTagsList to prevent stale entries - Add test coverage for legacy digest cleanup during tag deletion - Fix error handling in test (check ListTags error before asserting length)
- commit
- 96bcae541ec7781ec0bca2603ce6baebfd3dbff4
- author
- brice · · 2026-02-03 06:53 UTC
- parent
- 446fba0
- signature
- unsigned
2 files · +54 -1
M internal/adapters/out/filesystem/manifest.go +9 -0
| @@ -339,6 +339,15 @@ func (s *ManifestStorage) removeFromTagsList(name, reference string) error { | |||
| 339 | 339 | return err | |
| 340 | 340 | } | |
| 341 | 341 | ||
| 342 | + | // Drop any legacy digest entries before persisting | |
| 343 | + | filtered := make([]string, 0, len(tags)) | |
| 344 | + | for _, tag := range tags { | |
| 345 | + | if !validation.IsDigest(tag) { | |
| 346 | + | filtered = append(filtered, tag) | |
| 347 | + | } | |
| 348 | + | } | |
| 349 | + | tags = filtered | |
| 350 | + | ||
| 342 | 351 | // Remove tag if present | |
| 343 | 352 | var newTags []string | |
| 344 | 353 | for _, tag := range tags { | |
| @@ -339,6 +339,15 @@ func (s *ManifestStorage) removeFromTagsList(name, reference string) error { | |||||
| 339 | return err | 339 | return err | ||
| 340 | } | 340 | } | ||
| 341 | 341 | ||||
| 342 | + | // Drop any legacy digest entries before persisting | |||
| 343 | + | filtered := make([]string, 0, len(tags)) | |||
| 344 | + | for _, tag := range tags { | |||
| 345 | + | if !validation.IsDigest(tag) { | |||
| 346 | + | filtered = append(filtered, tag) | |||
| 347 | + | } | |||
| 348 | + | } | |||
| 349 | + | tags = filtered | |||
| 350 | + | ||||
| 342 | // Remove tag if present | 351 | // Remove tag if present | ||
| 343 | var newTags []string | 352 | var newTags []string | ||
| 344 | for _, tag := range tags { | 353 | for _, tag := range tags { | ||
M internal/adapters/out/filesystem/manifest_test.go +45 -1
| @@ -482,7 +482,8 @@ func TestManifestStorage_UpdateTagsList_CleansLegacyDigestEntries(t *testing.T) | |||
| 482 | 482 | require.NoError(t, err) | |
| 483 | 483 | ||
| 484 | 484 | // Verify tags.json contains digest entries | |
| 485 | - | tagsBefore, _ := storage.ListTags("myapp") | |
| 485 | + | tagsBefore, err := storage.ListTags("myapp") | |
| 486 | + | require.NoError(t, err) | |
| 486 | 487 | require.Len(t, tagsBefore, 4, "Should have 4 entries (2 tags + 2 digests)") | |
| 487 | 488 | ||
| 488 | 489 | // Add a new tag - this should trigger cleanup of legacy digest entries | |
| @@ -499,3 +500,46 @@ func TestManifestStorage_UpdateTagsList_CleansLegacyDigestEntries(t *testing.T) | |||
| 499 | 500 | assert.NotContains(t, tagsAfter, "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") | |
| 500 | 501 | assert.NotContains(t, tagsAfter, "sha256:a6b1c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") | |
| 501 | 502 | } | |
| 503 | + | ||
| 504 | + | func TestManifestStorage_RemoveFromTagsList_CleansLegacyDigestEntries(t *testing.T) { | |
| 505 | + | tmpDir := t.TempDir() | |
| 506 | + | log := testLogger() | |
| 507 | + | ||
| 508 | + | storage, err := NewManifestStorage(tmpDir, log) | |
| 509 | + | require.NoError(t, err) | |
| 510 | + | ||
| 511 | + | manifestData := []byte(`{"schemaVersion": 2}`) | |
| 512 | + | contentType := "application/vnd.docker.distribution.manifest.v2+json" | |
| 513 | + | ||
| 514 | + | // Create initial tags | |
| 515 | + | err = storage.PutManifest("myapp", "v1.0", contentType, manifestData) | |
| 516 | + | require.NoError(t, err) | |
| 517 | + | err = storage.PutManifest("myapp", "v2.0", contentType, manifestData) | |
| 518 | + | require.NoError(t, err) | |
| 519 | + | err = storage.PutManifest("myapp", "v3.0", contentType, manifestData) | |
| 520 | + | require.NoError(t, err) | |
| 521 | + | ||
| 522 | + | // Manually inject digest entries into tags.json to simulate legacy data | |
| 523 | + | tagsPath := filepath.Join(tmpDir, "repositories", "myapp", "tags.json") | |
| 524 | + | err = os.WriteFile(tagsPath, []byte(`["v1.0","v2.0","v3.0","sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855","sha256:a6b1c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"]`), 0600) | |
| 525 | + | require.NoError(t, err) | |
| 526 | + | ||
| 527 | + | // Verify tags.json contains digest entries | |
| 528 | + | tagsBefore, err := storage.ListTags("myapp") | |
| 529 | + | require.NoError(t, err) | |
| 530 | + | require.Len(t, tagsBefore, 5, "Should have 5 entries (3 tags + 2 digests)") | |
| 531 | + | ||
| 532 | + | // Delete a tag - this should trigger cleanup of legacy digest entries | |
| 533 | + | err = storage.DeleteManifest("myapp", "v2.0") | |
| 534 | + | require.NoError(t, err) | |
| 535 | + | ||
| 536 | + | // Verify that digest entries were cleaned up and v2.0 was removed | |
| 537 | + | tagsAfter, err := storage.ListTags("myapp") | |
| 538 | + | require.NoError(t, err) | |
| 539 | + | assert.Len(t, tagsAfter, 2, "Should have 2 entries (only remaining tags, digests removed)") | |
| 540 | + | assert.Contains(t, tagsAfter, "v1.0") | |
| 541 | + | assert.Contains(t, tagsAfter, "v3.0") | |
| 542 | + | assert.NotContains(t, tagsAfter, "v2.0") | |
| 543 | + | assert.NotContains(t, tagsAfter, "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") | |
| 544 | + | assert.NotContains(t, tagsAfter, "sha256:a6b1c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") | |
| 545 | + | } | |
| @@ -482,7 +482,8 @@ func TestManifestStorage_UpdateTagsList_CleansLegacyDigestEntries(t *testing.T) | |||||
| 482 | require.NoError(t, err) | 482 | require.NoError(t, err) | ||
| 483 | 483 | ||||
| 484 | // Verify tags.json contains digest entries | 484 | // Verify tags.json contains digest entries | ||
| 485 | - | tagsBefore, _ := storage.ListTags("myapp") | 485 | + | tagsBefore, err := storage.ListTags("myapp") |
| 486 | + | require.NoError(t, err) | |||
| 486 | require.Len(t, tagsBefore, 4, "Should have 4 entries (2 tags + 2 digests)") | 487 | require.Len(t, tagsBefore, 4, "Should have 4 entries (2 tags + 2 digests)") | ||
| 487 | 488 | ||||
| 488 | // Add a new tag - this should trigger cleanup of legacy digest entries | 489 | // Add a new tag - this should trigger cleanup of legacy digest entries | ||
| @@ -499,3 +500,46 @@ func TestManifestStorage_UpdateTagsList_CleansLegacyDigestEntries(t *testing.T) | |||||
| 499 | assert.NotContains(t, tagsAfter, "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") | 500 | assert.NotContains(t, tagsAfter, "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") | ||
| 500 | assert.NotContains(t, tagsAfter, "sha256:a6b1c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") | 501 | assert.NotContains(t, tagsAfter, "sha256:a6b1c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") | ||
| 501 | } | 502 | } | ||
| 503 | + | ||||
| 504 | + | func TestManifestStorage_RemoveFromTagsList_CleansLegacyDigestEntries(t *testing.T) { | |||
| 505 | + | tmpDir := t.TempDir() | |||
| 506 | + | log := testLogger() | |||
| 507 | + | ||||
| 508 | + | storage, err := NewManifestStorage(tmpDir, log) | |||
| 509 | + | require.NoError(t, err) | |||
| 510 | + | ||||
| 511 | + | manifestData := []byte(`{"schemaVersion": 2}`) | |||
| 512 | + | contentType := "application/vnd.docker.distribution.manifest.v2+json" | |||
| 513 | + | ||||
| 514 | + | // Create initial tags | |||
| 515 | + | err = storage.PutManifest("myapp", "v1.0", contentType, manifestData) | |||
| 516 | + | require.NoError(t, err) | |||
| 517 | + | err = storage.PutManifest("myapp", "v2.0", contentType, manifestData) | |||
| 518 | + | require.NoError(t, err) | |||
| 519 | + | err = storage.PutManifest("myapp", "v3.0", contentType, manifestData) | |||
| 520 | + | require.NoError(t, err) | |||
| 521 | + | ||||
| 522 | + | // Manually inject digest entries into tags.json to simulate legacy data | |||
| 523 | + | tagsPath := filepath.Join(tmpDir, "repositories", "myapp", "tags.json") | |||
| 524 | + | err = os.WriteFile(tagsPath, []byte(`["v1.0","v2.0","v3.0","sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855","sha256:a6b1c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"]`), 0600) | |||
| 525 | + | require.NoError(t, err) | |||
| 526 | + | ||||
| 527 | + | // Verify tags.json contains digest entries | |||
| 528 | + | tagsBefore, err := storage.ListTags("myapp") | |||
| 529 | + | require.NoError(t, err) | |||
| 530 | + | require.Len(t, tagsBefore, 5, "Should have 5 entries (3 tags + 2 digests)") | |||
| 531 | + | ||||
| 532 | + | // Delete a tag - this should trigger cleanup of legacy digest entries | |||
| 533 | + | err = storage.DeleteManifest("myapp", "v2.0") | |||
| 534 | + | require.NoError(t, err) | |||
| 535 | + | ||||
| 536 | + | // Verify that digest entries were cleaned up and v2.0 was removed | |||
| 537 | + | tagsAfter, err := storage.ListTags("myapp") | |||
| 538 | + | require.NoError(t, err) | |||
| 539 | + | assert.Len(t, tagsAfter, 2, "Should have 2 entries (only remaining tags, digests removed)") | |||
| 540 | + | assert.Contains(t, tagsAfter, "v1.0") | |||
| 541 | + | assert.Contains(t, tagsAfter, "v3.0") | |||
| 542 | + | assert.NotContains(t, tagsAfter, "v2.0") | |||
| 543 | + | assert.NotContains(t, tagsAfter, "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") | |||
| 544 | + | assert.NotContains(t, tagsAfter, "sha256:a6b1c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") | |||
| 545 | + | } | |||