Skip to content

Conversation

@ainsleyclark
Copy link
Contributor

Problem

The backup workflow template was creating backup jobs for ALL resources
regardless of their backup.enabled configuration. When a user set
backup.enabled: false, the job was still created in backup.yaml,
causing unnecessary CI/CD jobs to run.

Root Cause

While commit eacb38c added the IsBackupEnabled() method and infrastructure,
the template was never updated to check this flag before creating backup jobs.
The template only checked resource type (postgres, s3, sqlite) but not whether
backup was enabled.

Solution

Updated backup.yaml.tmpl to check .IsBackupEnabled before creating backup jobs:

  • Line 10: Added check for postgres resources
  • Line 113: Added check for s3 resources
  • Line 163: Added check for sqlite resources
  • Line 328: Updated sync-to-gdrive dependencies to only include enabled backups

Tests

Added comprehensive test coverage:

  • Test for single resource with backup disabled
  • Test for mixed enabled/disabled resources
  • Test for all resources with backup disabled
  • Verified backup jobs are only created when enabled
  • Verified sync-to-gdrive only depends on enabled backup jobs

Example

For this resource definition:

{
  "name": "store",
  "type": "s3",
  "backup": { "enabled": false }
}

Before: backup-resource-store job was created
After: No backup job created (as expected)

## Problem
The backup workflow template was creating backup jobs for ALL resources
regardless of their backup.enabled configuration. When a user set
backup.enabled: false, the job was still created in backup.yaml,
causing unnecessary CI/CD jobs to run.

## Root Cause
While commit eacb38c added the IsBackupEnabled() method and infrastructure,
the template was never updated to check this flag before creating backup jobs.
The template only checked resource type (postgres, s3, sqlite) but not whether
backup was enabled.

## Solution
Updated backup.yaml.tmpl to check .IsBackupEnabled before creating backup jobs:
- Line 10: Added check for postgres resources
- Line 113: Added check for s3 resources
- Line 163: Added check for sqlite resources
- Line 328: Updated sync-to-gdrive dependencies to only include enabled backups

## Tests
Added comprehensive test coverage:
- Test for single resource with backup disabled
- Test for mixed enabled/disabled resources
- Test for all resources with backup disabled
- Verified backup jobs are only created when enabled
- Verified sync-to-gdrive only depends on enabled backup jobs

## Example
For this resource definition:
```json
{
  "name": "store",
  "type": "s3",
  "backup": { "enabled": false }
}
```

Before: backup-resource-store job was created
After: No backup job created (as expected)
@claude

This comment has been minimized.

Move resource filtering logic from the backup.yaml.tmpl template layer to the Go layer in backup.go, following the established pattern in vm_maintenance.go. This improves code maintainability, testability, and separates concerns between business logic (Go) and presentation (templates).

Changes:
- Add filterBackupableResources() helper function to filter resources based on backup.enabled flag and provider compatibility (S3 must be DigitalOcean, SQLite must be Turso)
- Update BackupWorkflow to use filtered resources for secretData loop and template data
- Remove provider compatibility checks from secretData loop (now handled in Go via filterBackupableResources)
- Simplify backup.yaml.tmpl template by removing .IsBackupEnabled checks from conditional job creation (all resources in template are already backupable)
- Simplify sync-to-gdrive dependencies to iterate only over backupable resources
- All existing tests pass without modification, validating identical output

Benefits:
- Business logic in Go (easier to test and maintain)
- Template simpler with fewer conditionals
- Follows established codebase patterns
- 100% backward compatible

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@ainsleyclark ainsleyclark force-pushed the claude/setup-s3-storage-cdn-01KhtQPh3e4kF5B3Q8WEVKuT branch from 9655827 to 62c6118 Compare December 1, 2025 09:18
@codecov
Copy link

codecov bot commented Dec 1, 2025

Codecov Report

❌ Patch coverage is 80.95238% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.46%. Comparing base (7f6b060) to head (62c6118).
⚠️ Report is 421 commits behind head on main.

Files with missing lines Patch % Lines
internal/cmd/cicd/backup.go 80.95% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #342      +/-   ##
==========================================
+ Coverage   64.59%   69.46%   +4.87%     
==========================================
  Files         154      184      +30     
  Lines        6064     7277    +1213     
==========================================
+ Hits         3917     5055    +1138     
+ Misses       2064     2025      -39     
- Partials       83      197     +114     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ainsleyclark ainsleyclark merged commit fefe344 into main Dec 1, 2025
5 of 6 checks passed
@ainsleyclark ainsleyclark deleted the claude/setup-s3-storage-cdn-01KhtQPh3e4kF5B3Q8WEVKuT branch December 1, 2025 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants