open-policy-agent/frameworks

Use string constants consistently

willbeason opened this issue · 6 comments

Strings like the following should live in their canonical locations in kubernetes packages:

"v1alpha1"
"v1beta1"
"constraints.gatekeeper.sh"

This aids discoverability, makes testing easier, and ensures that differences in manually-typed strings are impossible.

goconst is a great tool for discovering these. Don't worry about getting every instance - just the ones that make sense/are important to flow.

go get github.com/jgautheron/goconst/cmd/goconst

goconst -ignore "remote" ./pkg/... | sort

Anything in client/drivers/remote is safe to ignore

The ones I've found are:

  • "v1"
  • "v1alpha1"
  • "v1beta1"
  • "constraints.gatekeeper.sh"
  • "templates.gatekeeper.sh"
  • "externaldata.gatekeeper.sh"

For the /api/ locations, the appropriate place is in a declaration like:

	SchemeGroupVersion = schema.GroupVersion{Group: "externaldata.gatekeeper.sh", Version: "v1alpha1"}

In some cases, we define a constant but the code doesn't use it. For example "ProviderRequest" has a definition in constraint/pkg/externaldata/request.go but it isn't referenced in the same file.

"data.lib" and "data.inventory" aren't repeated a bunch of times, but they're so important we should have them as constants so it's clear that their repetition in both "main.go" and "client.go" is meaningful and can't accidentally be broken.

That's everything I could find. There might be more (feel free to look if you want).

Also "data" since we define it as the root object users reference, so it pops up in a bunch of locations.

Is this a blocker for sharding?

Moving constants to pkg/apis seems reasonable

Mostly done as parts of various PRs.