Skip to content

Comments

chore: polish up state management#327

Open
brunodam wants to merge 5 commits intobll-state-managementfrom
00322-polish-up-state-management
Open

chore: polish up state management#327
brunodam wants to merge 5 commits intobll-state-managementfrom
00322-polish-up-state-management

Conversation

@brunodam
Copy link
Contributor

Description

This pull request introduces two main improvements: enhanced backward compatibility for configuration files and a fix for Helm integration with Kubernetes regarding field manager name length. It also adds comprehensive tests to ensure these changes work as intended.

Configuration Backward Compatibility:

  • Added automatic migration of deprecated config fields (release, chart) to their new names (releaseName, chartRepo) for backward compatibility, along with deprecation warnings. (internal/config/config.go [1] internal/config/migration.go [2]
  • Added tests to verify migration logic, correct handling of both old and new config fields, and precedence of new field names. (internal/config/config_test.go internal/config/config_test.goR58-R176)

Helm Field Manager Name Fix:

  • Implemented logic to truncate the Kubernetes field manager name used by Helm to a maximum of 128 bytes, preventing API rejections due to overly long names (especially when running tests or in certain IDEs). (pkg/helm/helpers.go [1] [2]
  • Added tests to ensure the truncation logic works correctly with various path lengths and real-world scenarios. (pkg/helm/helpers_test.go pkg/helm/helpers_test.goR1-R83)

Minor Fixes:

Related Issues

Signed-off-by: Bruno De Assis Marques <bruno.marques@swirldslabs.com>
@brunodam brunodam requested a review from a team as a code owner January 21, 2026 06:08
@brunodam brunodam requested review from Copilot and crypto-pablo and removed request for crypto-pablo January 21, 2026 06:08
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request enhances backward compatibility for configuration files and fixes a Helm integration issue with Kubernetes field manager name length constraints. It also corrects a minor test setup bug.

Changes:

  • Implements automatic migration of deprecated config fields (release, chart) to new names (releaseName, chartRepo) with deprecation warnings
  • Adds field manager name truncation logic to prevent Kubernetes API rejections when running tests or in certain IDEs
  • Fixes incorrect parameter type in test setup by dereferencing the pointer

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
internal/config/migration.go Adds backward compatibility logic for migrating deprecated config field names with deprecation warnings
internal/config/config.go Integrates migration logic into the configuration initialization process
internal/config/config_test.go Adds comprehensive tests for config migration including old fields, new fields, and precedence
pkg/helm/helpers.go Implements field manager name truncation to comply with Kubernetes 128-byte limit
pkg/helm/helpers_test.go Adds tests for field manager truncation logic
internal/workflows/steps/step_setup_directories_test.go Fixes test by correctly dereferencing WeaverPaths pointer

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Bruno De Assis Marques <bruno.marques@swirldslabs.com>
@brunodam brunodam force-pushed the 00322-polish-up-state-management branch from 3cf0534 to 136c77e Compare January 21, 2026 06:39
Signed-off-by: Bruno De Assis Marques <bruno.marques@swirldslabs.com>
@brunodam brunodam force-pushed the 00322-polish-up-state-management branch 4 times, most recently from e0b4be0 to 4a3e989 Compare January 22, 2026 00:05
Signed-off-by: Bruno De Assis Marques <bruno.marques@swirldslabs.com>
@brunodam brunodam force-pushed the 00322-polish-up-state-management branch from 4a3e989 to 5b52168 Compare January 22, 2026 00:34
Signed-off-by: Bruno De Assis Marques <bruno.marques@swirldslabs.com>
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.

2 participants