Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config: Allow configuration of capabilities #423

Merged
merged 7 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 87 additions & 2 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,97 @@ import (

"gopkg.in/yaml.v3"

"github.com/open-policy-agent/opa/ast"

rio "github.com/styrainc/regal/internal/io"
)

const capabilitiesEngineOPA = "opa"

type Config struct {
Rules map[string]Category `json:"rules" yaml:"rules"`
Ignore Ignore `json:"ignore,omitempty" yaml:"ignore,omitempty"`
Rules map[string]Category `json:"rules" yaml:"rules"`
Ignore Ignore `json:"ignore,omitempty" yaml:"ignore,omitempty"`
Capabilities ast.Capabilities `json:"capabilities,omitempty" yaml:"capabilities,omitempty"`
}

func (config *Config) UnmarshalYAML(value *yaml.Node) error {
var result struct {
Rules map[string]Category `yaml:"rules"`
Ignore Ignore `yaml:"ignore"`
Capabilities struct {
From struct {
Engine string `yaml:"engine"`
Version string `yaml:"version"`
File string `yaml:"file"`
} `yaml:"from"`
Plus struct {
Builtins []*ast.Builtin `yaml:"builtins"`
} `yaml:"plus"`
Minus struct {
Builtins []struct {
Name string `yaml:"name"`
} `yaml:"builtins"`
} `yaml:"minus"`
} `yaml:"capabilities"`
}

if err := value.Decode(&result); err != nil {
return fmt.Errorf("unmarshalling config failed %w", err)
}

config.Rules = result.Rules
config.Ignore = result.Ignore

capabilitiesFile := result.Capabilities.From.File
capabilitiesEngine := result.Capabilities.From.Engine
capabilitiesEngineVersion := result.Capabilities.From.Version

if capabilitiesFile != "" && capabilitiesEngine != "" {
return fmt.Errorf("capabilities from.file and from.engine are mutually exclusive")
}

if capabilitiesEngine != "" && capabilitiesEngineVersion == "" {
return fmt.Errorf("please set the version for the engine from which to load capabilities from")
}

if capabilitiesFile != "" {
bs, err := os.ReadFile(capabilitiesFile)
if err != nil {
return fmt.Errorf("failed to load capabilities file: %w", err)
}

err = json.Unmarshal(bs, &config.Capabilities)
if err != nil {
return fmt.Errorf("failed to unmarshal capabilities file contents: %w", err)
}
}

if capabilitiesEngine != "" && result.Capabilities.From.Engine == capabilitiesEngineOPA {
capabilities, err := ast.LoadCapabilitiesVersion(result.Capabilities.From.Version)
if err != nil {
return fmt.Errorf("loading capabilities failed: %w", err)
}

config.Capabilities = *capabilities
}

// by default, use the capabilities from the current OPA
if capabilitiesEngine == "" && capabilitiesFile == "" {
config.Capabilities = *ast.CapabilitiesForThisVersion()
}

// remove any builtins referenced in the minus config
for i, builtin := range config.Capabilities.Builtins {
for _, minusBuiltin := range result.Capabilities.Minus.Builtins {
if minusBuiltin.Name == builtin.Name {
config.Capabilities.Builtins = append(config.Capabilities.Builtins[:i], config.Capabilities.Builtins[i+1:]...)
}
}
}

config.Capabilities.Builtins = append(config.Capabilities.Builtins, result.Capabilities.Plus.Builtins...)

return nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If no capabilities have been provided, should we default to latest from OPA here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've had a go a finishing this off a little more in 1fff993

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks 😃 Looks good to me.

The one thing I'm not sure about is passing the capabilities from the config to Rego's eval args. Would that not have the linter (i.e. Regal) itself be limited to whatever capabilities the target policy has? I mean, even if we're linting a Rego file with e.g. a pre-contains OPA version, surely we should be able to use contains in Regal's linter policy?

I think the way we'll want to use the capabilities at least initially, is basically as a datasource for what built-in functions we might encounter in the policy we're linting, and have linter rules be able to declare their dependencies... like how we should skip the custom-has-key-construct if object.keys is not in the capabilities, and so on.

But if we eval with the provided capabilities, it would mean we can't use object.keys, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, I see. I think I got the wrong end of the stick! I can take this part out.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've taken out the changes in 5d79a7b and then added the defaulting back in d292b58

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

}

type Category map[string]Rule
Expand Down
133 changes: 133 additions & 0 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"gopkg.in/yaml.v3"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/util/test"

rio "github.com/styrainc/regal/internal/io"
Expand Down Expand Up @@ -123,6 +124,22 @@ func TestUnmarshalConfig(t *testing.T) {
files:
- foo.rego
level: error
capabilities:
from:
engine: opa
version: v0.45.0
plus:
builtins:
- name: ldap.query
type: function
decl:
args:
- type: string
result:
type: object
minus:
builtins:
- name: http.send
`)

var conf Config
Expand Down Expand Up @@ -158,4 +175,120 @@ func TestUnmarshalConfig(t *testing.T) {
if conf.Rules["testing"]["foo"].Extra["level"] != nil {
t.Errorf("expected extra attribute 'level' to be removed")
}

if exp, got := 183, len(conf.Capabilities.Builtins); exp != got {
t.Errorf("expected %d builtins, got %d", exp, got)
}

expectedBuiltins := []string{"regex.match", "ldap.query"}

for _, expectedBuiltin := range expectedBuiltins {
expectedBuiltinFound := false

for _, bi := range conf.Capabilities.Builtins {
if bi.Name == expectedBuiltin {
expectedBuiltinFound = true

break
}
}

if !expectedBuiltinFound {
t.Errorf("expected builtin %s to be found", expectedBuiltin)
}
}

unexpectedBuiltins := []string{"http.send"}

for _, unexpectedBuiltin := range unexpectedBuiltins {
unexpectedBuiltinFound := false

for _, bi := range conf.Capabilities.Builtins {
if bi.Name == unexpectedBuiltin {
unexpectedBuiltinFound = true

break
}
}

if unexpectedBuiltinFound {
t.Errorf("expected builtin %s to be removed", unexpectedBuiltin)
}
}
}

func TestUnmarshalConfigWithBuiltinsFile(t *testing.T) {
t.Parallel()

bs := []byte(`rules: {}
capabilities:
from:
file: "./fixtures/caps.json"
`)

var conf Config

if err := yaml.Unmarshal(bs, &conf); err != nil {
t.Fatal(err)
}

if exp, got := 1, len(conf.Capabilities.Builtins); exp != got {
t.Errorf("expected %d builtins, got %d", exp, got)
}

expectedBuiltins := []string{"wow"}

for _, expectedBuiltin := range expectedBuiltins {
expectedBuiltinFound := false

for _, bi := range conf.Capabilities.Builtins {
if bi.Name == expectedBuiltin {
expectedBuiltinFound = true

break
}
}

if !expectedBuiltinFound {
t.Errorf("expected builtin %s to be found", expectedBuiltin)
}
}
}

func TestUnmarshalConfigDefaultCapabilities(t *testing.T) {
t.Parallel()

bs := []byte(`rules: {}
`)

var conf Config

if err := yaml.Unmarshal(bs, &conf); err != nil {
t.Fatal(err)
}

caps := ast.CapabilitiesForThisVersion()

if exp, got := len(caps.Builtins), len(conf.Capabilities.Builtins); exp != got {
t.Errorf("expected %d builtins, got %d", exp, got)
}

// choose the first built-ins to check for to keep the test fast
expectedBuiltins := []string{caps.Builtins[0].Name, caps.Builtins[1].Name}

for _, expectedBuiltin := range expectedBuiltins {
expectedBuiltinFound := false

for _, bi := range conf.Capabilities.Builtins {
if bi.Name == expectedBuiltin {
expectedBuiltinFound = true

break
}
}

if !expectedBuiltinFound {
t.Errorf("expected builtin %s to be found", expectedBuiltin)
}
}
}
35 changes: 35 additions & 0 deletions pkg/config/fixtures/caps.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"builtins": [
{
"name": "wow",
"description": "Increases wow in Rego rule",
"categories": [
"special"
],
"decl": {
"args": [
{
"name": "x",
"type": "number"
}
],
"result": {
"description": "the wowness level for input `x`",
"name": "y",
"type": "number"
},
"type": "function"
}
}
],
"future_keywords": [
"contains",
"every",
"if",
"in"
],
"wasm_abi_versions": null,
"features": [
"rule_head_ref_string_prefixes"
]
}
Loading