Skip to content

Make config-remote-sync patching logic more generic - #4400

Merged
ilyakuz-db merged 11 commits into
mainfrom
split-confing-sync
Jan 30, 2026
Merged

Make config-remote-sync patching logic more generic#4400
ilyakuz-db merged 11 commits into
mainfrom
split-confing-sync

Conversation

@ilyakuz-db

@ilyakuz-dbilyakuz-db commented Jan 29, 2026

Copy link
Copy Markdown
Contributor

Changes

Split the config-remote-sync command into 3 separate phases. Previously, some DAB-specific logic was embedded in the patcher, now it

  1. Detect (diff.go) - Compare states, handle server-side and CLI defaults. Convert value to the internal struct. Compute which operation type should be applied to the config

  2. Resolve (resolve.go) - Determine file locations and field paths. Replace [task_key=foo] selectors

  3. Patch (patch.go) - Now only handles YAML modifications, doesn't hold any config-specific logic

Changes:

  1. new ConfigChangeDesc struct
  2. normalizeValue() moved from patch.go to diff.go
  3. selector handling moved to resolve.go
  4. /targets/ prefixes moved from patch.go to resolve.go
  5. new explicit type for operations which we also show in the output
  6. removed bunch of redundant tests - this logic is covered in acceptance suites

Why

Preparation before the next PR, where I plan to properly handle CLI defaults

Tests

  1. Updated snapshots to capture new debug output
  2. Removed redundant unit tests - these cases are already covered with acceptance tests

@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Jan 29, 2026

Copy link
Copy Markdown
Collaborator

Commit: b694b69

Run: 21512118775

Env🟨​KNOWN💚​RECOVERED🙈​SKIP✅​pass🙈​skipTime
🟨​aws linux71744270824:29
🟨​aws windows71741471621:11
🟨​aws-ucws linux58564157890:43
🟨​aws-ucws windows58561058785:39
💚​azure linux2944270722:14
💚​azure windows2941471521:54
🟨​azure-ucws linux34763757786:03
🟨​azure-ucws windows34760658681:44
💚​gcp linux2943171322:09
💚​gcp windows2940372119:40
18 interesting tests: 9 KNOWN, 5 SKIP, 4 RECOVERED
Test Nameaws linuxaws windowsaws-ucws linuxaws-ucws windowsazure linuxazure windowsazure-ucws linuxazure-ucws windowsgcp linuxgcp windows
🟨​TestAccept🟨​K🟨​K🟨​K🟨​K💚​R💚​R🟨​K🟨​K💚​R💚​R
🙈​TestAccept/bundle/deployment/bind/alert🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/bundle/generate/alert🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🟨​TestAccept/bundle/invariant/no_drift🙈​S🙈​S🟨​K🟨​K🙈​S🙈​S🟨​K🟨​K🙈​S🙈​S
🟨​TestAccept/bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=alert.yml.tmpl🟨​K🟨​K🟨​K🟨​K
🙈​TestAccept/bundle/resources/alerts/basic🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/bundle/resources/alerts/with_file🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/bundle/resources/permissions🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🟨​TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions🟨​K🟨​K🟨​K🟨​K🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🟨​TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=direct🟨​K🟨​K🟨​K🟨​K
🟨​TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=terraform🟨​K🟨​K💚​R💚​R
🟨​TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions🟨​K🟨​K💚​R💚​R🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🟨​TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=direct🟨​K🟨​K💚​R💚​R
🟨​TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=terraform🟨​K🟨​K💚​R💚​R
💚​TestAccept/bundle/resources/synced_database_tables/basic🙈​S🙈​S💚​R💚​R🙈​S🙈​S💚​R💚​R🙈​S🙈​S
💚​TestAccept/bundle/resources/synced_database_tables/basic/DATABRICKS_BUNDLE_ENGINE=direct💚​R💚​R💚​R💚​R
💚​TestAccept/bundle/resources/synced_database_tables/basic/DATABRICKS_BUNDLE_ENGINE=terraform💚​R💚​R💚​R💚​R
💚​TestAccept/ssh/connection💚​R💚​R💚​R💚​R💚​R💚​R💚​R💚​R💚​R💚​R
Top 50 slowest tests (at least 2 minutes):
durationenvtestname
7:59azure-ucws windowsTestAccept/bundle/resources/synced_database_tables/basic/DATABRICKS_BUNDLE_ENGINE=direct
7:57aws-ucws windowsTestAccept/bundle/resources/synced_database_tables/basic/DATABRICKS_BUNDLE_ENGINE=terraform
7:26aws-ucws linuxTestAccept/bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=synced_database_table.yml.tmpl
7:10aws-ucws linuxTestAccept/bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=database_instance.yml.tmpl
7:05aws-ucws linuxTestAccept/bundle/resources/synced_database_tables/basic/DATABRICKS_BUNDLE_ENGINE=direct
7:00aws-ucws windowsTestAccept/bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=database_catalog.yml.tmpl
6:55aws-ucws linuxTestAccept/bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=database_catalog.yml.tmpl
6:38aws linuxTestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
6:22aws windowsTestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
6:18aws-ucws windowsTestAccept/bundle/resources/synced_database_tables/basic/DATABRICKS_BUNDLE_ENGINE=direct
6:11azure-ucws linuxTestAccept/bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=database_catalog.yml.tmpl
6:09aws-ucws linuxTestAccept/bundle/resources/synced_database_tables/basic/DATABRICKS_BUNDLE_ENGINE=terraform
6:06aws-ucws windowsTestAccept/bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=database_instance.yml.tmpl
6:02aws-ucws windowsTestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform
5:50aws-ucws windowsTestAccept/bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=synced_database_table.yml.tmpl
5:47azure-ucws linuxTestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform
5:47aws-ucws windowsTestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
5:44aws linuxTestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform
5:34gcp linuxTestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform
5:24azure-ucws linuxTestAccept/bundle/resources/synced_database_tables/basic/DATABRICKS_BUNDLE_ENGINE=terraform
5:18gcp windowsTestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform
5:17gcp linuxTestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
4:50azure-ucws windowsTestAccept/bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=synced_database_table.yml.tmpl
4:45azure-ucws windowsTestAccept/bundle/resources/synced_database_tables/basic/DATABRICKS_BUNDLE_ENGINE=terraform
4:43azure-ucws linuxTestAccept/bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=synced_database_table.yml.tmpl
4:09azure-ucws windowsTestAccept/bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=database_catalog.yml.tmpl
4:07azure windowsTestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
4:00azure-ucws windowsTestAccept/bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=database_instance.yml.tmpl
3:42azure windowsTestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform
3:24azure-ucws linuxTestAccept/bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=database_instance.yml.tmpl
3:16gcp linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:16gcp windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:16azure-ucws linuxTestAccept/bundle/resources/synced_database_tables/basic/DATABRICKS_BUNDLE_ENGINE=direct
3:15gcp linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:13gcp windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:58azure-ucws linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:57aws-ucws linuxTestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
2:56aws-ucws linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:53aws-ucws linuxTestAccept/bundle/templates/default-python/combinations/classic/DATABRICKS_BUNDLE_ENGINE=terraform/DLT=yes/NBOOK=yes/PY=yes/READPLAN=
2:52azure-ucws windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:44aws-ucws windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:44aws windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:44aws-ucws windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:42aws linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:34aws-ucws linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:31azure-ucws linuxTestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct
2:23azure linuxTestAccept
2:22azure-ucws linuxTestAccept/bundle/templates/default-python/combinations/classic/DATABRICKS_BUNDLE_ENGINE=terraform/DLT=no/NBOOK=yes/PY=no/READPLAN=
2:22gcp linuxTestAccept
2:19aws windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct


// normalizeValue converts values to plain Go types suitable for YAML patching
// by using SDK marshaling which properly handles ForceSendFields and other annotations.
func normalizeValue(v any) (any, error) {

@ilyakuz-dbilyakuz-dbJan 30, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This is moved as is from patch.go

Will check if it's needed and can it be replaced with dyn in following PRs


type resolvedChanges map[string]*deployplan.ChangeDesc
// ApplyChangesToYAML generates YAML files for the given field changes.
func ApplyChangesToYAML(ctx context.Context, b *bundle.Bundle, fieldChanges []FieldChange) ([]FileChange, error) {

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This was simplified to reduce nesting level, also bundle-specific logic was moved to diff.go and resolve.go

}

// ResolveChanges resolves selectors and computes field path candidates for each change.
func ResolveChanges(ctx context.Context, b *bundle.Bundle, configChanges Changes) ([]FieldChange, error) {

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

All path resolution is handled here now, I will update this to traverse path nodes in following PRs

@denikdenik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you make PR description more concrete? Which functions/types were moved, which were changed, which replaced by something newer.

@ilyakuz-db
ilyakuz-db marked this pull request as ready for review January 30, 2026 15:26
@ilyakuz-db

Copy link
Copy Markdown
ContributorAuthor

Could you make PR description more concrete? Which functions/types were moved, which were changed, which replaced by something newer.

Elaborated a bit

@ilyakuz-db
ilyakuz-db added this pull request to the merge queueJan 30, 2026
Merged via the queue into main with commit e46c3fbJan 30, 2026
23 checks passed
@ilyakuz-db
ilyakuz-db deleted the split-confing-sync branch January 30, 2026 15:29
pietern added a commit that referenced this pull request Apr 13, 2026
PR #4353 migrated direct imports to go.yaml.in/yaml/v3. PR #4400
added palantir/pkg/yamlpatch a week later, reintroducing
gopkg.in/yaml.v3 as a standalone indirect require line. Move it
into the grouped indirect block via go mod tidy.
Co-authored-by: Isaac
github-merge-queueBot pushed a commit that referenced this pull request Apr 13, 2026
## Summary
- Move `gopkg.in/yaml.v3` from a standalone indirect `require` line into
the grouped indirect block via `go mod tidy`
- #4353 migrated direct imports to `go.yaml.in/yaml/v3` and moved
`gopkg.in/yaml.v3` into the grouped indirect block
- #4289 added a direct dependency on `gopkg.in/yaml.v3` (via
`palantir/pkg/yamlpatch`), pulling it back out as a standalone `require`
line
- #4400 marked it as `// indirect` but left it as a standalone line
## Test plan
- [x] `go mod tidy` produces no diff
This pull request was AI-assisted by Isaac.
denik pushed a commit that referenced this pull request May 20, 2026
## Changes
Split the config-remote-sync command into 3 separate phases. Previously,
some DAB-specific logic was embedded in the patcher, now it
1. Detect (diff.go) - Compare states, handle server-side and CLI
defaults. Convert value to the internal struct. Compute which operation
type should be applied to the config
2. Resolve (resolve.go) - Determine file locations and field paths.
Replace [task_key=foo] selectors
3. Patch (patch.go) - Now only handles YAML modifications, doesn't hold
any config-specific logic
Changes:
1. new ConfigChangeDesc struct 2. normalizeValue() moved from patch.go to diff.go
3. selector handling moved to `resolve.go` 4. /targets/ prefixes moved from `patch.go` to `resolve.go` 5. new explicit type for operations which we also show in the output
6. removed bunch of redundant tests - this logic is covered in
acceptance suites
## Why
Preparation before the next PR, where I plan to properly handle CLI
defaults
## Tests
1. Updated snapshots to capture new debug output
2. Removed redundant unit tests - these cases are already covered with
acceptance tests
<!-- If your PR needs to be included in the release notes for next
release,
add a separate entry in NEXT_CHANGELOG.md as part of your PR. -->
denik pushed a commit that referenced this pull request May 20, 2026
## Summary
- Move `gopkg.in/yaml.v3` from a standalone indirect `require` line into
the grouped indirect block via `go mod tidy`
- #4353 migrated direct imports to `go.yaml.in/yaml/v3` and moved
`gopkg.in/yaml.v3` into the grouped indirect block
- #4289 added a direct dependency on `gopkg.in/yaml.v3` (via
`palantir/pkg/yamlpatch`), pulling it back out as a standalone `require`
line
- #4400 marked it as `// indirect` but left it as a standalone line
## Test plan
- [x] `go mod tidy` produces no diff
This pull request was AI-assisted by Isaac.
Sign up for freeto 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

@ilyakuz-db@eng-dev-ecosystem-bot@denik