* Secrets manager: create secure value using the active keeper
* SecureValueService.Update: fetch secure value from db to get the keeper
* fix keeper_store_test.go
* SecureValueService: fix bug in update where the current version keeper wasn't being passed to the createNewVersion method
* make gofmt
* remove outdated test
* update TestModel
* undo enterprise_imports changes
* use xkube.Namespace
* migrator: set secret_secure_value.keeper to 'system' when the column is null
* indent cue
* fix tests
* fix enterprise imports
* properly fix enterprise imports
* make update-workspace
* go mod tidy
---------
Co-authored-by: Matheus Macabu <macabu.matheus@gmail.com>
* Add library panel repeat options to v2 schema during conversion
* use any instead of interface{}
* change to common.Unstructured instead of byte[] for model field
* Fix the tests and let the library panel behavior fetch repeat options in public and scripted dashboards
* fix library panel differences between backend and frontend conversion
* `grafana-iam`: Use the UniStore as the default store
* Refactor all instantiations
* Remove enableDualWriter
* Nit. dw is clear enough
* Use the correct access control client
* feat: add retry logic for transient errors in Kubernetes client
Add retry wrapper for dynamic.ResourceInterface that automatically retries
transient errors using Kubernetes' wait.ExponentialBackoff utility.
- Implements retry logic with exponential backoff for all Kubernetes API operations
- Detects transient errors: ServiceUnavailable, ServerTimeout, TooManyRequests,
InternalError, Timeout, and network errors
- Uses wait.ExponentialBackoff from k8s.io/apimachinery/pkg/util/wait
- Respects context cancellation
- Includes comprehensive unit tests
Part of https://github.com/grafana/git-ui-sync-project/issues/634
* docs: add detailed documentation for defaultRetryBackoff
Document when retry attempts will happen, what errors trigger retries,
and the retry behavior (attempts, delays, exponential backoff, jitter).
* feat: add logging and increase retry attempts for Kubernetes client
- Add context logger to track retry attempts (Info for retries, Warn for exhaustion)
- Increase retry attempts from 5 to 8 steps (~10 seconds total retry window)
- Document when all retry attempts will fail:
* API server completely unavailable/unreachable
* Network connectivity issues persist beyond retry window
* Consistent transient errors for entire retry duration
* Context cancellation before retries complete
* chore: update retry client documentation
* fix: resolve linting issues in retry client
- Replace type assertions with errors.As for wrapped errors
- Remove deprecated Temporary() check (deprecated since Go 1.18)
- Update tests to remove temporary error test case
* fix: resolve staticcheck S1008 linting issue in retry_client.go
Simplify return statement to use errors.As directly instead of if-return pattern
* passes nsr to GetResourceStats instead of just namespace
* removes ns check
* fixes failing tests
* make update-workspace
* pass group and resource from rebuild request when getting resource stats
* Provisioning: Remove image renderer note from PR comment template
Removes the 'NOTE: The image renderer is not configured' message from
the pull request comment template when image renderer is unavailable.
This addresses issue #656 in git-ui-sync-project.
- Updated commentTemplateMissingImageRenderer to be empty
- Updated testdata to reflect the change
- All unit tests pass
* Provisioning: Make image renderer note optional in PR comments
Make the image renderer note in pull request comments optional based on
the allowImageRendering configuration flag. When enabled, the note now
includes a link to the setup documentation.
- Add showImageRendererNote boolean field to commenter struct
- Update NewCommenter to accept showImageRendererNote parameter
- Update template to conditionally show note with documentation link
- Pass allowImageRendering from APIBuilder to commenter in register.go
- Update ProvidePullRequestWorker to use cfg.ProvisioningAllowImageRendering
- Add tests to verify note appears/disappears based on flag
Fixes https://github.com/grafana/git-ui-sync-project/issues/656
* Format code with go fmt
* Remove redundant text from image renderer note
Remove 'The image renderer is not configured.' from the note message.
The note now focuses on actionable guidance with the documentation link.
* Fix compilation error: use cfg.ProvisioningAllowImageRendering directly
Cannot access unexported field allowImageRendering from webhooks package.
Use cfg.ProvisioningAllowImageRendering directly since we have access to cfg.
* provisioning: detect stale sync status and trigger resync
When sync jobs expire and are cleaned up by the expired job cleanup
controller, the Repository sync status remains stuck in Pending or
Working state. This prevents new sync jobs from being queued because
shouldResync() blocks on these states.
This change adds detection logic in shouldResync() to check if a sync
job referenced in the sync status still exists. If the job doesn't exist
(NotFound), we trigger a resync to reconcile the stale state.
Fixesgrafana/git-ui-sync-project#626
* test: remove unused mocks and fix test case
- Remove unused mockRepositoryLister and mockRepositoryNamespaceLister types
- Remove unused imports (labels, listers)
- Remove test case for sync disabled scenario as we don't care about sync enabled state when detecting stale status
* refactor: Move job cleanup to separate controller and fix history write
- Created JobCleanupController in apps/provisioning/pkg/controller
- Separated cleanup logic from ConcurrentJobDriver
- Fixed bug where expired jobs were not written to history
- Added comprehensive tests with 93.8% coverage
- Removed cleanup interval parameter from ConcurrentJobDriver
- Cleanup now properly follows Complete + WriteJob pattern
Fixes expired jobs being lost instead of archived
* refactor: Update lease renewal interval to use jobExpiry variable
- Changed the lease renewal interval in the GetPostStartHooks method to utilize the jobExpiry variable for improved clarity and maintainability.
* Format code
* Fix Unix milliseconds
* fix: correct Unix timestamp assertions and remove duplicate test expectations
- Changed Unix() to UnixMilli() for correct millisecond timestamp validation
- Removed duplicate store.AssertExpectations(t) calls throughout tests
* Provisioning: Improve logging and tracing in job processing
- Add comprehensive tracing with OpenTelemetry spans across all job operations
- Enhance logging with consistent style: lowercase, concise messages, appropriate log levels
- Use past tense for completed lifecycle events (e.g., 'stopped' vs 'stop')
- Add structured logging with contextual attributes for better searchability
- Handle graceful shutdowns without throwing errors on context cancellation
- Refactor Cleanup method into listExpiredJobs and cleanUpExpiredJob for better code quality
- Avoid double logging by only logging errors when handled locally
- Add tracing and logging to historyjob controller cleanup operations
Files modified:
- pkg/registry/apis/provisioning/jobs/driver.go: Add tracing spans and improve error handling for graceful shutdown
- pkg/registry/apis/provisioning/jobs/concurrent_driver.go: Add tracing and consistent logging
- pkg/registry/apis/provisioning/jobs/persistentstore.go: Add comprehensive tracing and logging to all public methods, refactor cleanup
- apps/provisioning/pkg/controller/historyjob.go: Add tracing and improve logging consistency
* Update pkg/registry/apis/provisioning/jobs/persistentstore.go
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Refactor logging in persistentstore.go
- Remove debug log statements at the start of job operations for cleaner output
- Maintain structured logging with contextual attributes for improved traceability
Files modified:
- pkg/registry/apis/provisioning/jobs/persistentstore.go: Clean up logging for job operations
* Enhance logging and tracing in provisioning job operations
- Introduce OpenTelemetry spans for better observability in job processing and webhook handling
- Improve structured logging with contextual attributes for key operations
- Remove unnecessary tracing spans in long-running functions to streamline performance
- Update error handling to record errors in spans for better traceability
Files modified:
- pkg/registry/apis/provisioning/controller/repository.go: Add tracing and structured logging to sync job operations
- pkg/registry/apis/provisioning/jobs/concurrent_driver.go: Remove tracing span from long-running function
- pkg/registry/apis/provisioning/jobs/driver.go: Enhance logging and tracing in job processing
- pkg/registry/apis/provisioning/webhooks/webhook.go: Implement tracing and structured logging for webhook connections
* Update pkg/registry/apis/provisioning/jobs/driver.go
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Improve error handling in ConcurrentJobDriver to differentiate between graceful shutdown and unexpected stops
* Remove unused import in driver.go to clean up code
---------
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
The FlagGrafanaAPIServerWithExperimentalAPIs is only available when
`app_mode=development`. We have a more specific flag that is usable in
production, so use that.
Also, there was some old code constraining these APIs to a static list
of datasources. We don't need that anymore, so this PR removes it.
The FlagQueryServiceWithConnections is left as is, because there are
multiple existing tests that rely on this development-only, experimental
flag. I don't want to understand why that is.
Fix: Include ref field in DELETE endpoint response for branch operations
When deleting a dashboard file via DELETE endpoint with a ref query parameter
(for branch operations), the response was missing the ref field. This caused
the frontend branch workflow success handler to fail silently.
The issue was an inverted boolean condition in the Delete method. The code
was setting file.Ref = opts.Ref when shouldUpdateGrafanaDB returned true
(main branch operations), but it should have been setting it when false
(branch operations), since we read the file with an empty ref.
Fixed by inverting the condition from:
if r.shouldUpdateGrafanaDB(opts, nil)
to:
if !r.shouldUpdateGrafanaDB(opts, nil)
This ensures the ref field is properly included in the ResourceWrapper
response for branch operations.
* Validate Job Specs
* Add comprehensive unit test coverage for job validator
- Added 8 new test cases to improve coverage from 88.9% to ~100%
- Tests for migrate action without options
- Tests for delete/move actions with resources (missing kind)
- Tests for move action with valid resources
- Tests for move/delete with both paths and resources
- Tests for move action with invalid source paths
- Tests for push action with valid paths
Now covers all validation paths including resource validation and
edge cases for all job action types.
* Add integration tests for job validation
Added comprehensive integration tests that verify the job validator properly
rejects invalid job specifications via the API:
- Test job without action (required field)
- Test job with invalid action
- Test pull job without pull options
- Test push job without push options
- Test push job with invalid branch name (consecutive dots)
- Test push job with path traversal attempt
- Test delete job without paths or resources
- Test delete job with invalid path (path traversal)
- Test move job without target path
- Test move job without paths or resources
- Test move job with invalid target path (path traversal)
- Test migrate job without migrate options
- Test valid pull job to ensure validation doesn't block legitimate requests
These tests verify that the admission controller properly validates job specs
before they are persisted, ensuring security (path traversal prevention) and
data integrity (required fields/options).
* Remove valid job test case from integration tests
Removed the positive test case as it's not necessary for validation testing.
The integration tests now focus solely on verifying that invalid job specs
are properly rejected by the admission controller.
* Fix movejob_test to expect validation error at creation time
Updated the 'move without target path' test to expect the job creation
to fail with a validation error, rather than expecting the job to be
created and then fail during execution.
This aligns with the new job validation logic which rejects invalid
job specs at the API admission control level (422 Unprocessable Entity)
before they can be persisted.
This is better behavior as it prevents invalid jobs from being created
in the first place, rather than allowing them to be created and then
failing during execution.
* Simplify action validation using slices.Contains
Replaced manual loop with slices.Contains for cleaner, more idiomatic Go code.
This reduces code complexity while maintaining the same validation logic.
- Added import for 'slices' package
- Replaced 8-line loop with 1-line slices.Contains call
- All unit tests pass
* Refactor job action validation in ValidateJob function
Removed the hardcoded valid actions array and simplified the validation logic. The function now directly appends an error for invalid actions, improving code clarity and maintainability. This change aligns with the recent updates to job validation, ensuring that invalid job specifications are properly handled.
* Provisioning: Preserve in progress job data
* Refactor code and cover more situations
* Fix linting
* Fix issue with remove path operation for started time
* Cleanup
* prettier
---------
Co-authored-by: Roberto Jimenez Sanchez <roberto.jimenez@grafana.com>
* cleanup
* library panel via search
* test cleanup
* merge main
* add FindDashboards mock
* no matching dashbaords should return empty
* do not alllow name and libraryPanel query
* Provisioning: allow access check to proceed even when non access policy
* Provisioning: access checker needs this for MT
* add permissions registration
* remove scopes
* use in MT for now
* no need to document an internal flag here
* revert vscode change
* refactor the authZ permission evaluation and mapper code to allow evaluating unscoped actions beyond creation
* update wire
* gofmt
* add boolean to struct
---------
Co-authored-by: IevaVasiljeva <ieva.vasiljeva@grafana.com>
BrunoandMatheus Macabu
Oscar Kilhed
Peter Štibraný
Mustafa Sencer ÖzcanandRafael Paulovic
Gabriel MABILLE
Roberto Jiménez Sánchez
Charandas
Dave Henderson
Jean-Philippe Quéméner
Misi
Ryan McKinley
owensmallwood
Gábor Farkas
Costa Alexoglou
beejeebus
Alexander Zobnin
Ezequiel Victorero
Jo
Alex KhomenkoandRoberto Jimenez Sanchez
