And enable more staticcheck linters. I saw staticcheck failures
mentioned in another PR, so thought I'd check it out.
- `WriteString(fmt.Sprintf)` -> `fmt.Fprintf`
- Rewrite calls to deprecated `*Rule.Path()`
- Don't use `==` to compare `time.Time`
- Use inline ignores over config exclusions of paths
- Remove 'varcheck' ignores as no longer used
- Remove v0 topdown/graphql.go (!)
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
Add a separate field to keep track of the parsed min and max interval keeping the original fields to represent the user provided values. This prevents multiple calls to config.ValidateAndInjectDefaults from overflowing the values but still makes sure that the value has been converted at least once.
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
- Bump golangci-lint -> 2.6.2
- Fix all `deprecatedComment` "notices should be in a dedicated paragraph, separated from the rest" reports
- Enable `appendCombine` and fix all "appendCombine: can combine chain of X appends into one" notices
- Enable `preferFprint` and fix the few reported issues
- Fix various issues reported only once or twice, like `zeroByteRepeat`
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
Updates the Bundle Plugins oneShot callback function signature used by Downloader, OCIDownloader, and fileLoader to return an error. This allows any issues in the callback function such as Rego parsing issues to be returned.
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Go 1.23 is no longer supported as per Go release policy.
Changes:
- Use Go v1.24.6 as the project SDK requirement
- Apply lint fixes for Go 1.24
- Fix "non-constant format string in call" issues as seen in CI.
Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
* v1/plugins: Address race in config access
I ran into this race condition on another PR:
https://github.com/open-policy-agent/opa/actions/runs/16655603110/job/47139789057
I have tried to make all manager.Config access thread-safe by adding new
getters for used values. GetConfig is regrettably based on a JSON
roundtrip deep copy of the config. This us used in tests (fine) but also
in the discovery plugin:
https://github.com/open-policy-agent/opa/blob/2d014a89bbbc307d7204817220146ffae992e838/v1/plugins/discovery/discovery.go#L122
getPluginSet is very tightly coupled to the manager.Config and because
of it's dependencies on status and the other plugins packages, it's hard
to break out.
So, for now, I think this is an improvement and worth getting a second
opinion on before more refactoring.
Signed-off-by: Charlie Egan <charlie@styra.com>
* v1/config: Use add Clone to config
This makes the use of the manager's config more thread-safe and
consistent without more API changes.
Signed-off-by: Charlie Egan <charlie@styra.com>
* topdown: Add clone() funcs for config structs
NamedValueCacheConfig.Clone, InterQueryBuiltinValueCacheConfig.Clone and
InterQueryBuiltinCacheConfig.Clone have been added.
All Clone methods return a deep copy of the struct. This is tested for
missed new fields using PopulateAllFields, a generic function that
stuffs structs with values for all fields.
Signed-off-by: Charlie Egan <charlie@styra.com>
* plugins: Clone new config
Signed-off-by: Charlie Egan <charlie@styra.com>
---------
Signed-off-by: Charlie Egan <charlie@styra.com>
* plugin/bundle: Correct bundle delay behavior
I ran into an issue when testing an earlier change:
https://github.com/open-policy-agent/opa/actions/runs/16646981900/job/47110035165
I found that this test generated around 100,000 lines of errors showing
the bundle downloader running.
This can be be tested using:
```
go test -v ./v1/plugins/bundle -count=1 2>&1 | grep -c "request failed"
```
This commit closes managers and plugins correctly.
Signed-off-by: Charlie Egan <charlie@styra.com>
* download: Update stop to be idempotent
I had some race detector issues with TestStartStopWithLongPollNotSupported
https://github.com/open-policy-agent/opa/actions/runs/16722869930/job/47334690407?pr=7812
I think this is a deadlock around multiple calls to Stop dead locking
updating the stopped var.
Signed-off-by: Charlie Egan <charlie@styra.com>
---------
Signed-off-by: Charlie Egan <charlie@styra.com>
Following up on #7566, and now applying the more exciting
modernizations. fmt.Appendf was new to me! But especially
the contains checks are so much better IMHO. I have reviewed
all changes myself and did a few manual changes where it
became obvious that things could be improved a little further.
(the modernize analyzer still has some issues running against
OPA, and I have manually worked around those for the time being)
Signed-off-by: Anders Eknert <anders@styra.com>
Brace yourselves! For there are many touched files here. No changes
in semantics however.
Spent a long time trying out the various optional rules gocritic
provides, and settled for a few of them. There are more I really
like, but that would take many hours to address across the codebase.
Perhaps others find gocritic too pedantic? If so, we can merge the
fixes without enabling the rule.
Signed-off-by: Anders Eknert <anders@styra.com>
By tagging the worst offenders, we can make use of `go test -short` to
avoid them for a quicker dev-test cycle. Compare:
```
make test 200.69s user 209.81s system 170% cpu 4:01.20 total
```
```
make test-short 70.32s user 29.17s system 350% cpu 28.367 total
```
From 4 minutes down to under 30 seconds. The short tests can either
be run with `go test -short ./...` or `make test-short`.
We'll still run the full test suite in CI, naturally.
Also:
- Remove section on benchmarking that linked to a no longer used resource.
Signed-off-by: Anders Eknert <anders@styra.com>
where the bundle has been manually constructed containing v0 Rego modules and
no `rego_version`/`file_rego_versions` fields are declared in the bundle manifest.
This affects bundle deactivation in the bundle store lifecycle used when for
`opa run` in server mode (`-s`).
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
And update code to conform to the rule.
- Replace unnecessary fmt.Sprintf with string concatenation
- Replace fmt.Sprint with more efficient strconv.Itoa
- Replace static fmt.Errorf calls with more efficient errors.New
Thanks @srenatus for pushing me down this rabbit hole!
Signed-off-by: Anders Eknert <anders@styra.com>
I pushed most of these fixed previously but these ones required
more work. Probably more work that it was worth, lol, but now the
work is done... and at least we can add this to the list of enabled
checks.
Fixes#7238
Signed-off-by: Anders Eknert <anders@styra.com>
Fixing issue where bundle plugin would panic on reconfiguration if module rego-version is missing in bundle manifest.
* Passing runtime rego-version to deactivation options
* Preferring to pull rego-version from parsed modules if present
This solves an edge case when using the OPA SDK, and should not affect standalone OPA.
Fixes: #7297
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
Fixing an issue where the rego-version for individual modules was lost during bundle deactivation (bundle lifecycle) if this version diverged from the active runtime rego-version. This could cause reloading of v0 bundles to fail when OPA was not running with the `--v0-compatible` flag.
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
Co-authored-by: Johan Fylling <johan.dev@fylling.se>
And a few other small fixes in tests. This i not so much
about performance but about choosing the best tool for a
given task :) But that the alternatives are also faster
doesn't hurt either.
Signed-off-by: Anders Eknert <anders@styra.com>
Anders Eknert