* Add some unit tests and enable sub resource so we can start testing manually on dev
* add test to verify auth headers are stripped when being forwarding requests to client
* add integration tests
* update test packages
* polish
* fix tests
* fix panic
* fmt
* white space
* add feature flag to routing of resources endpoint
* fix missing type
* update integration tests with correct feature flag
* fix test
* update other integration tests to use ff
test: Add integration tests for empty path repositories
Add comprehensive integration tests to verify empty path repository behavior:
- Single repository with empty path syncs from repository root
- Multiple repositories with empty path can coexist
- Dashboard ownership conflicts are properly detected and reported
- Folders are duplicated per repository (12 total for 2 repos)
- Dashboards remain unique to first repository (3 total)
This validates the feature that allows repositories to use empty paths
for syncing from the root directory, while properly handling resource
ownership conflicts between multiple repositories.
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
* Simplify unified storage client backend setup
* Gate storage backend creation by storage type
* Allow unified grpc storage without backend
* refactor(unified): ProvideStorageBackend before unified service
* refactor(unified): implement ProvideStorageBackend and provide backend before service
* Fix issues after merge
* simplify changes
* fix missing reference
* fix tests
* fix lint and add comment to NewStorageBackend
* start service in test
* separate module for unified backend
* Stop unified backend after grpc
* Fix tests
* Shutdown backend last
* Do not reply on ishealthy after shutdown started
* Cleanup a bit the code
* Init backend at register time
* Do not change health checks for now
* re-add storage metrics
* check for nil on testinfra sql.NewStorageBackend
* add tracer for backend and set max_open_conn in test
* address claude review
* make distributor an idle server
* ensure server is created in test
* Add dry-run support to app platform storage operations
What
- Added dry-run check in folder_storage.go to skip permission side effects during dry-run operations
- Added dry-run handling in dualwriter.go for Create, Delete, Update, and DeleteCollection methods
- When dry-run flag is set, operations delegate directly to unified storage which properly handles dry-run via DryRunnableStorage
- Added comprehensive test suite in dualwriter_dryrun_test.go covering all dual-writer modes (1-3) for all operation types
- Added integration test in folders_test.go (TestIntegrationFolderDryRun) validating dry-run behavior across modes
Why
- Dry-run operations must not have side effects (no permissions, no actual writes)
- Unified storage already implements correct dry-run semantics via DryRunnableStorage
- Legacy storage does not properly handle dry-run, so bypassing it ensures correctness
- Aligns dualwriter behavior with Kubernetes API semantics for dry-run requests
- Ensures data consistency and prevents unintended state changes during dry-run validation
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
* DualWriter: Wrap objInfo in Update dry-run path for Mode1/Mode2
What:
- Apply wrappedUpdateInfo translation in the Update dry-run path when
readUnified is false (Mode1/Mode2), matching the normal path behavior
- Add tests verifying wrapping behavior per mode
Why:
- In Mode1/Mode2, clients read from legacy storage and send legacy
UID/resourceVersion. The dry-run path was sending these directly to
unified storage, which would fail with precondition mismatches. The
normal path already wraps objInfo to clear UID/RV/preconditions for the
secondary store — the dry-run path must do the same.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
* provisioning: check quota on files API requests
* provisioning: add integration test for files quota checks
* provisioning: delete AI generated comment that's not adding much
* provisioning: add quota check before syncing
* provisioning: add unit and integration tests to job quota checks
* provisioning: make full sync to pre-check
* provisioning: cleanup incremental changes
* provsioning: make quota exceeded a warning and avoid updating lastref in that situation
* provisioning: integration tests expect a warning
* provisioning: simplify quota check in full jobs and allow deletion only syncs
* Allow replacing extra configurations in Alertmanager
* Added a `replace` flag to `SaveAndApplyExtraConfiguration` to enable replacing existing configurations.
* Introduced a new header, `X-Grafana-Alerting-Config-Force-Replace`, to trigger forced replacement.
* Updated relevant tests to cover the new replace logic.
* Add dry-run support to SaveAndApplyExtraConfiguration
* Introduced `dryRun` flag to `SaveAndApplyExtraConfiguration` to validate configurations without saving them.
* Added `X-Grafana-Alerting-Dry-Run` header for API usage.
* Updated tests to include dry-run functionality.
* return renamed resources in response
* update integration tests
* fix: Strip BOM characters in dashboard provisioning and admission
Fixes "illegal byte order mark" errors during repository deletion when
dashboards contain Byte Order Mark (BOM) characters.
## Problem
- Frontend links editor can add BOM characters to dashboard links
- Git files may contain BOMs from certain text editors
- Dashboards with BOMs fail validation during PATCH operations
- Repository deletion fails when trying to remove ownership annotations
## Solution
Implement multi-layer BOM stripping:
1. **File parsing layer** (provisioning):
- Strip BOMs when reading JSON/YAML files from Git repositories
- Prevents BOMs from being stored initially
2. **Admission mutation layer** (all operations):
- Strip BOMs during Create/Update/Patch for all dashboard versions
- Remediates existing dashboards with BOMs during any modification
- Handles v0alpha1, v1beta1, v2alpha1, and v2beta1 formats
## Changes
- Add BOM utility functions to pkg/util/strings.go:
- StripBOM() - removes BOM from strings
- StripBOMFromBytes() - handles UTF-8 BOM prefix and embedded BOMs
- StripBOMFromInterface() - recursively strips BOMs from nested structures
- Update provisioning file readers:
- ReadClassicResource() - strip BOMs before JSON parsing
- DecodeYAMLObject() - strip BOMs before YAML decoding
- Update dashboard admission mutation for all versions:
- v0alpha1/v1beta1: direct BOM stripping on Unstructured specs
- v2alpha1/v2beta1: marshal/unmarshal BOM stripping on typed specs
## Testing
- Added comprehensive unit tests for BOM utility functions
- Added mutation tests for BOM stripping across all dashboard versions
- All existing tests continue to pass
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Add BOM cleanup utilities and integration tests
Adds comprehensive tooling for dealing with existing dashboards that
contain BOMs in production environments.
## New Integration Tests
- Test UTF-8 BOM prefix in JSON files
- Test embedded Unicode BOMs in JSON strings
- Test BOMs in YAML files
- Test BOMs in nested structures (panels, titles, etc.)
- Enhanced file parsing to strip BOMs from parsed JSON/YAML values
## Cleanup Utilities
### Shell Script (scripts/cleanup_bom_dashboards.sh)
- Scans all dashboard versions for BOMs
- Patches dashboards to trigger admission mutation cleanup
- Supports dry-run mode for safe testing
- Works with label selectors for targeted cleanup
### Go Utilities (pkg/registry/apis/dashboard/bom_cleanup.go)
- BOM detection functions for all dashboard versions
- Field path tracking to identify exact locations of BOMs
- Framework for programmatic cleanup operations
- Statistics tracking for cleanup operations
### Documentation (docs/sources/dashboards/bom-cleanup.md)
- Comprehensive guide for BOM cleanup strategies
- Passive approach: automatic cleanup during updates
- Active approaches: kubectl script, K8s Job, Git Sync
- Troubleshooting guide and FAQ
## Enhanced File Parsing
Updated fileformat.go to strip BOMs from parsed JSON/YAML values:
- ReadClassicResource: strips BOMs after JSON unmarshal
- DecodeYAMLObject: strips BOMs after YAML decode
- Handles both file-level BOMs and embedded string BOMs
## Use Cases
**Immediate**: Run cleanup script to fix existing dashboards
**Ongoing**: Automatic cleanup via admission mutation
**Monitoring**: Detection utilities to assess scope of problem
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Replace shell script with proper integration tests
Removes the shell script and adds comprehensive Go integration tests
in the proper test directories as requested.
## Integration Tests Added
### Dashboard Integration Tests (pkg/tests/apis/dashboard/bom_test.go)
- **Create with BOMs**: Tests dashboard creation with BOMs in title, description, and panels
- **Patch with BOMs**: Tests patching dashboards to remove annotations (simulates repository deletion)
- **Update with BOMs**: Tests updating dashboards that have BOMs
- **JSON with embedded BOMs**: Tests BOMs in raw JSON payloads
- Tests both v0alpha1 and v1beta1 dashboard versions
- Uses full K8s test environment with real clients
### Provisioning Integration Tests (pkg/tests/apis/provisioning/bom_test.go)
- **UTF-8 BOM prefix**: Tests dashboard files with UTF-8 BOM bytes (EF BB BF)
- **Embedded BOMs in strings**: Tests BOMs in JSON string values
- **Repository deletion**: Tests the original error scenario end-to-end
- **YAML with BOMs**: Tests YAML files with BOM prefixes
- Full provisioning flow: Git file → Parse → Create dashboard → Verify
- Tests repository deletion triggering dashboard patches
## What These Tests Verify
1. **File Parsing Layer**: BOMs stripped when reading from Git repos
2. **Admission Mutation Layer**: BOMs stripped during Create/Update/Patch
3. **End-to-End Flow**: Git file with BOM → provisioned dashboard without BOM
4. **Original Bug**: Repository deletion succeeds even with BOM dashboards
## Test Coverage
- ✅ All dashboard versions (v0alpha1, v1beta1)
- ✅ All operations (Create, Update, Patch)
- ✅ All BOM types (UTF-8 prefix, embedded Unicode)
- ✅ All file formats (JSON, YAML)
- ✅ Repository deletion scenario (the original bug)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Remove dashboard API changes - provisioning only
Per feedback, this PR now focuses ONLY on the provisioning layer:
- ✅ File parsing BOM stripping (when reading from Git repos)
- ❌ NO dashboard API admission mutation changes
- ❌ NO dashboard API integration tests
## What's in this PR Now
**Provisioning Layer Only**:
1. `pkg/util/strings.go` - BOM utility functions
2. `pkg/util/strings_test.go` - Unit tests for utilities
3. `pkg/registry/apis/provisioning/resources/fileformat.go` - Strip BOMs when parsing files
4. `pkg/registry/apis/provisioning/resources/fileformat_test.go` - Unit tests for file parsing
5. `pkg/tests/apis/provisioning/bom_test.go` - Integration tests for provisioning flow
6. `docs/sources/dashboards/bom-cleanup.md` - Documentation
## What This Fixes
**Prevention**: Dashboards provisioned from Git will NOT have BOMs
- BOMs stripped when reading JSON/YAML files
- BOMs stripped from parsed values
**What's NOT Fixed**:
- Dashboards created directly via dashboard API (not through provisioning)
- Existing dashboards that already have BOMs
- PATCH operations on dashboards with BOMs
The provisioning fix prevents new BOMs from being introduced via Git Sync.
Dashboard API changes can be addressed separately if needed.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Add BOM stripping in repository finalizer for v0 dashboards
Fixes the "illegal byte order mark" error during repository deletion
by stripping BOMs from v0alpha1 dashboards before patching them to
remove ownership annotations.
## Problem
When a repository is deleted, the finalizer patches dashboards to
remove ownership annotations. If a dashboard has BOMs in its spec
(from before the provisioning fix), the PATCH operation fails with:
"illegal byte order mark" validation error.
## Solution
In `releaseResources()`, before patching to remove annotations:
1. Check if the resource is a v0alpha1 dashboard
2. Read the dashboard and check for BOMs in the spec
3. If BOMs found, strip them and update the dashboard
4. Then proceed with the annotation removal patch
This is done specifically for v0 dashboards since:
- v0 has no validation, so BOMs don't cause issues during creation
- But they cause validation errors during PATCH operations
- v1/v2 dashboards have validation that should catch BOMs earlier
## Implementation Details
- `stripBOMsFromDashboard()` - reads dashboard, strips BOMs, updates
- `containsBOM()` - recursively checks for BOM characters
- Best-effort approach: if BOM stripping fails, log warning and continue
- Only processes dashboards (not folders or other resources)
## Flow
```
Repository Deletion
↓
Finalizer: ReleaseOrphanResourcesFinalizer
↓
For each dashboard:
1. stripBOMsFromDashboard() (if v0)
2. PATCH to remove annotations
↓
Success ✅
```
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Optimize finalizer: Get once, strip BOMs + remove annotations together
Per feedback, optimize the finalizer to avoid redundant operations:
- Before: Get → Update (strip BOMs) → Patch (remove annotations)
- After: Get → Update (strip BOMs AND remove annotations together)
## Changes
- `releaseResourceWithBOMStripping()` - does both operations in single Update
- Get the dashboard once (we already need it to check for BOMs)
- Strip BOMs from spec (if present)
- Remove ownership annotations
- Single Update call with both changes
## Benefits
- More efficient: one Get + one Update instead of Get + Update + Patch
- Cleaner: all dashboard modifications in one place
- Safer: atomic operation for both changes
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Remove dashboard API cleanup files - provisioning only scope
These files were related to dashboard API cleanup utilities and documentation,
which are out of scope for this provisioning-focused PR.
* Remove stuff in finalizer
* Simplify provisioning finalizer - remove BOM workaround
Remove ~82 lines of special BOM handling code from the provisioning finalizer.
The dashboard mutation hook already strips BOMs during CREATE and UPDATE operations.
Since Kubernetes PATCH operations use admission.Update internally, the mutation
hook handles BOM stripping automatically during PATCH operations.
Changes:
- Remove releaseResourceWithBOMStripping() function
- Remove containsBOM() helper function
- Remove special v0alpha1 dashboard handling in releaseResources()
- Use standard PATCH for all resources
- Update integration test to verify finalizer releases resources correctly
The key integration test now:
- Adds release-orphan-resources finalizer to repository
- Deletes repository
- Verifies dashboards remain (not deleted)
- Verifies ownership annotations are removed (released)
- Confirms no 'illegal byte order mark' errors during PATCH
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Revert changes in dashboards
* Fix linting errors in bom_test.go
Add explicit error ignore for os.Remove in cleanup functions to satisfy
errcheck linter.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
* Initial reconciler implementation
* Refactor how zanzana gets the k8s client to list crd's
* Add toggle to disbale RBAC reconciler
* Make auth work with iam-apiserver
* Fix authorizer for standalone iam api server
* Add rolebinding storage to the standalone iam api server
* Increase unifi store page size to 10000
* Remove duplicate time based reconciliation logic
* Implement mt-reconciler running in embedded zanzana
* Consolidate reconciliation interval configuration for zanzana reconcilers
* Revert unrelated changes
* Re-word docs
* pre-allocate tuples where possible.
* Add translators test
* Fix user name
* remove hard-coded GVR's
* Make queue size configurable
* add some integration tests for existing datasource crud endpoints
* implement GET by uid redirect handler
* fix apis authorizer
* add unit tests for connections client
* add tests for the k8s datasource handler
* use the correct group, and dont prettify group name in listConnections response
* Provisioning: Wire up repo and connection factory in controllers
* addressing comments - updating the way extras are built in operators
* addressing comments
* re-adding a switch case
Updated GitHub repository validation to accept any HTTPS GitHub server
(github.com, GitHub Enterprise, etc.) instead of restricting to github.com only.
Changes:
- Modified validator to only require https:// prefix instead of https://github.com/
- Updated unit test to validate non-HTTPS rejection instead of non-github.com
- Added unit test for GitHub Enterprise URL validation
- Added integration test case for GitHub Enterprise URL acceptance
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
* fix(library-panels): cache folder tree for get all
* chore: re-add commented test
* Make cache global and fallback to service if folder is not found
* fix linter
* remove leftover comment and add adjust accessible checks
* Add integration tests for viewer repository read permissions
This commit adds integration tests to verify that viewers can now
read and list repositories following the change that granted the
repositoriesReader role to org.RoleViewer.
Changes:
- Update existing test to verify viewers CAN read repositories (previously tested for 403 Forbidden)
- Add new test to verify viewers can list repositories
- Verify that viewers see the same repository data as admins
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Fix repository authorization to allow viewer read access
The repository CRUD authorization was hardcoded to use accessWithAdmin
for all operations, which overrode the permissions defined in
accesscontrol.go. This prevented viewers from accessing repositories
even though repositoriesReader was granted to org.RoleViewer.
Changed to use verb-based authorization:
- Read operations (get, list, watch): accessWithViewer
- Write operations (create, update, patch, delete): accessWithAdmin
This allows viewers to read repositories while maintaining admin-only
write access, matching the permissions defined in accesscontrol.go.
Fixes the integration test failures where viewers were being denied
access despite having the correct role grants.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
* Provisioning: Treat validation errors as warnings instead of errors
Changes validation errors in provisioning jobs to be logged as warnings
instead of errors. This provides better UX by distinguishing between
user-correctable content issues and actual system errors.
## Changes
- Extended `isWarningError()` to recognize:
- Kubernetes field validation errors (e.g., missing resource name)
- Dashboard refresh interval validation errors
- Updated `Record()` method to log warnings at WARN level with message
"job resource operation completed with warning"
- Added comprehensive test coverage for both error types
## Impact
Jobs that previously failed with ERROR state due to malformed content
will now complete with WARNING state, making it easier to identify
which resources need correction without treating them as system failures.
Fixes: https://github.com/grafana/git-ui-sync-project/issues/818
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Refactor: Wrap validation errors at source for better architecture
Improved the implementation to wrap validation errors with
ResourceValidationError at the source (in the resources/parser layer)
rather than checking for specific error types in the progress recorder.
## Changes
### Resources Layer (`resources.go`, `parser.go`)
- Added `wrapAsValidationErrorIfNeeded()` helper function to wrap:
- Kubernetes field validation errors
- Dashboard refresh interval errors
- Wraps `ErrMissingName` with ResourceValidationError when returned
- Wraps errors from `parsed.Run()` that are validation-related
### Jobs Layer (`job_resource_result.go`)
- Simplified `isWarningError()` - now only checks for:
- ResourceValidationError
- ResourceOwnershipConflictError
- Removed specific checks for field.Error and DashboardErr
- Validation errors are now handled generically
## Benefits
- More maintainable: validation error handling is centralized
- Better architecture: errors are classified at the source
- Easier to extend: new validation errors just need wrapping
- Cleaner code: fewer dependencies and type checks
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Add integration tests for validation error warnings
Added integration tests to verify that specific validation errors
are properly treated as warnings:
1. Missing name validation error test
2. Dashboard refresh interval validation error test
These tests ensure end-to-end behavior matches expectations and
provide regression coverage for the validation error handling.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Revert unnecessary changes in job_resource_result files
Since validation errors are now wrapped at the source (in the resources
layer), we don't need the specific error type checks and tests in the
job layer. The integration tests provide end-to-end coverage.
This simplifies the code and removes unnecessary dependencies.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Add logger tests and expand validation error handling
## Logger Tests
- Added testLogger implementation to verify correct log levels
- Added tests to verify warnings logged at WARN level (not ERROR)
- Added tests to verify errors logged at ERROR level
- Added tests to verify success logged at INFO level
- Ensures alerts won't fire for user validation errors
## Expanded Validation Error Handling
- Wrap ALL dashboard errors as validation errors (not just specific ones)
- Wrap ErrDuplicateName as validation error
- Wrap ErrAlreadyInRepository as validation error
- These are user content errors, not system failures
## Code Style
- Grouped variable declarations with var ( in progress.go
This prevents alerting on user mistakes and content issues.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Fix dashboard validation error handling in integration tests
Dashboard validation errors (like refresh interval too low) are wrapped
as Kubernetes BadRequest errors by the dashboard API layer before
reaching our validation error wrapping code.
Updated wrapAsValidationErrorIfNeeded to also wrap BadRequest errors
as validation errors, since these are typically user content validation
issues, not system failures.
This ensures the integration tests pass and dashboard validation errors
are logged at WARN level instead of ERROR level.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
* [WIP] Provisioning: Get default branch for /test endpoint
* allowing empty branch only for gh repo, getting default branch in /test and repo controller
* adding integration tests
* adding unit tests for GetRepository
* [WIP] Provisioning: Get default branch for /test endpoint
* allowing empty branch only for gh repo, getting default branch in /test and repo controller
* adding integration tests
* adding unit tests for GetRepository
* updating validator
* removing logs from integration tests, using patcher for spec
* Wait for the repo reconciliation before fetching branches; remove default path and branch
* Updatae tests
---------
Co-authored-by: Clarity-89 <homes89@ukr.net>
* Include imported extra config in routes API as managed route
* Use identifier directly for imported route name
* Linting
* Use new helper methods to rename references in imported route
* Add spec.external to teambinding field selectors
* Add integration tests
* Update query for legacy store
* Fix test
* Use the new method for registering fieldselectors
* update workspace
When `config.featureToggles.queryServiceWithConnections` is true, calls
to `getDataSourceByUid` are routed through a new frontend function,
`getDataSourceFromK8sAPI`, which exercises the new app platform
kubernetes style APIs.
