upstream/grafana
1
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2026-02-19 02:30:53 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 68
grafana/pkg/util
History
Roberto Jiménez Sánchez c9a0d9e2dd
Provisioning: Strip BOM characters from files in parser (#117984) 
* 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>
2026-02-12 16:40:25 +00:00
..
cmd
cmputil
debouncer
errhttp
httpclient
osutil
proxyutil
retryer
ring
scheduler
sqlite
testutil Add non-boolean feature flags support to the StaticProvider (#115085) 2026-01-12 22:53:23 +01:00
xorm Zanzana: Refactor server provider (#116756) 2026-02-03 17:28:26 +01:00
contextutil.go
contextutil_test.go
encoding.go
encoding_test.go
encryption.go
encryption_test.go
interface.go
interface_test.go
ip_address.go
ip_address_test.go
json.go
json_test.go
md5.go
md5_test.go
pointer.go
reverse.go
reverse_test.go
shortid_generator.go
shortid_generator_race_test.go
shortid_generator_test.go
split_email.go
split_email_test.go
strings.go Provisioning: Strip BOM characters from files in parser (#117984) 2026-02-12 16:40:25 +00:00
strings_test.go Provisioning: Strip BOM characters from files in parser (#117984) 2026-02-12 16:40:25 +00:00
svg.go
svg_test.go
tls.go
tls_test.go
uri_sanitize.go
uri_sanitize_test.go
url.go
url_test.go
validation.go
validation_test.go