From e8935487986882e6360c8b9de3b4b08d747e2e2f Mon Sep 17 00:00:00 2001 From: Charles Daniels Date: Thu, 29 Aug 2024 11:06:07 -0400 Subject: [PATCH] implement PR review suggestions --- README.md | 44 +++++++++---------- build/do.rq | 40 +++++++++++------ .../capabilities_integration_test.go | 30 +++++++++++++ internal/capabilities/capabilities_test.go | 19 -------- 4 files changed, 79 insertions(+), 54 deletions(-) create mode 100644 internal/capabilities/capabilities_integration_test.go diff --git a/README.md b/README.md index 94860716..4a82785d 100644 --- a/README.md +++ b/README.md @@ -365,28 +365,6 @@ reached. If no configuration file is found, Regal will use the default configura A custom configuration may be also be provided using the `--config-file`/`-c` option for `regal lint`, which when provided will be used to override the default configuration. -### Loading Capabilities from URLs - -Starting with Regal version TODO, Regal can load capabilities from URLs with -the `file`, `http`, or `https` schemes using the `capabilities.from.url` config -key. For example, to load capabilities from -`https://example.org/capabilities.json`, this configuration could be used: - -```yaml -capabilities: - from: - url: https://example.org/capabilities.json -``` - -### Supported Engines - -Regal includes capabilities files for the following engines: - -| Engine | Website | Description | -|--------|---------|-------------| -| `opa` | [OPA website](https://www.openpolicyagent.org/) | Open Policy Agent | -| `eopa` | [Enterprise OPA website](https://www.styra.com/enterprise-opa/) | Styra Enterprise OPA | - ## Ignoring Rules If one of Regal's rules doesn't align with your team's preferences, don't worry! Regal is not meant to be the law, @@ -588,6 +566,28 @@ capabilities: type: object ``` +### Loading Capabilities from URLs + +Starting with Regal version TODO, Regal can load capabilities from URLs with +the `http`, or `https` schemes using the `capabilities.from.url` config key. +For example, to load capabilities from `https://example.org/capabilities.json`, +this configuration could be used: + +```yaml +capabilities: + from: + url: https://example.org/capabilities.json +``` + +### Supported Engines + +Regal includes capabilities files for the following engines: + +| Engine | Website | Description | +|--------|---------|-------------| +| `opa` | [OPA website](https://www.openpolicyagent.org/) | Open Policy Agent | +| `eopa` | [Enterprise OPA website](https://www.styra.com/enterprise-opa/) | Styra Enterprise OPA | + ## Exit Codes Exit codes are used to indicate the result of the `lint` command. The `--fail-level` provided for `regal lint` may be diff --git a/build/do.rq b/build/do.rq index 55100995..c952639a 100755 --- a/build/do.rq +++ b/build/do.rq @@ -162,6 +162,7 @@ test { e2e { run("go test -tags e2e ./e2e") + run("go test -tags integration ./internal/capabilities") } lint { @@ -236,13 +237,12 @@ fetch_eopa_caps { } ) - { rq.error(sprintf("\nstdout: %s\nstderr: %s\n", [eopa_tags_result.stdout, eopa_tags_result.stderr])) | eopa_tags_result.exitcode != 0 } - + error_nonzero(eopa_tags_result, "failed to fetch tags from GitHub") # We assume that tags and capabilities files are 1:1, but some EOPA # release tags in the past did not correctly get capabilities files, so # we eliminate them from consideration. - + known_bad_tags := { "v1.15.0", # tag missing capabilities file (misnamed v0.15.0) "v1.4.1", # tag missing capabilities file @@ -254,13 +254,26 @@ fetch_eopa_caps { # they really have a need, they can manually download them and use the # file locally. - eopa_tags := {t | some r in eopa_tags_result.stdout; t := r[2] ; not known_bad_tags[t] ; not startswith(t, "v0.")} + eopa_tags := { + t + | + some r in eopa_tags_result.stdout + t := r[2] + not known_bad_tags[t] + not startswith(t, "v0.") + } # Get a directory listing for the capabilities directory, filtering for # only nonzero size files with JSON extensions. The size check is to # avoid long-tail edge cases where we crashed after opening the file # for writing but before committing any content. - eopa_caps_tree := {p: f | f := rq.tree(eopa_caps_dir, {})[p]; f.size != 0; f.ext == "json"} + eopa_caps_tree := { + p: f + | + f := rq.tree(eopa_caps_dir, {})[p] + f.size != 0 + f.ext == "json" + } # Determine which capabilities files are missing, what URL they # should be fetched from, and where they should end up on disk. @@ -273,10 +286,7 @@ fetch_eopa_caps { t := eopa_tags[_] p := rq.joinpath([eopa_caps_dir, sprintf("%s.json", [t])]) - # eopa_caps_tree has object values, so if the path is missing, - # retrieving a non-object type is sufficient to check for - # nonexistance - object.get(eopa_caps_tree, p, "-") == "-" + not eopa_caps_tree[p] # construct the URL to fetch the content from r := rq.template("https://raw.githubusercontent.com/StyraInc/enterprise-opa/main/capabilities/{{.tag}}.json", {"tag": t}) @@ -291,13 +301,10 @@ fetch_eopa_caps { m := missing_locally[_] print("\tfetching ", m.remote) resp := http.send({"url": m.remote, "method": "GET"}) - + { rq.error(sprintf("non-200 status code '%d' for URL '%s'", [resp.status_code, m.remote])) | resp.status_code != 200 } c := resp.raw_body - - # sanity check in case we got the URL wrong - not regex.match("404: Not Found", c) } # Commit the retrieved content to disk. @@ -372,3 +379,10 @@ github(what, j) { } else := true is_github if rq.env().GITHUB_ACTION + +error_nonzero(run_result, message) if { + run_result.exitcode != 0 + rq.error(sprintf("%s\nstdout:%s\nstderr:\n%s\n", [message, run_result.stdout, run_result.stderr])) +} else { + true +} diff --git a/internal/capabilities/capabilities_integration_test.go b/internal/capabilities/capabilities_integration_test.go new file mode 100644 index 00000000..ec36b019 --- /dev/null +++ b/internal/capabilities/capabilities_integration_test.go @@ -0,0 +1,30 @@ +//go:build integration +// +build integration + +package capabilities + +import ( + "context" + "testing" +) + +// Since this test requires internet access, we hide it behind a flag. + +func TestLookupFromURL(t *testing.T) { + t.Parallel() + + // Test that we can load a one of the existing OPA capabilities files + // via GitHub. + + caps, err := Lookup( + context.Background(), + "https://raw.githubusercontent.com/open-policy-agent/opa/main/capabilities/v0.55.0.json", + ) + if err != nil { + t.Errorf("unexpected error from Lookup: %v", err) + } + + if len(caps.Builtins) != 193 { + t.Errorf("OPA v0.55.0 capabilities should have 193 builtins, not %d", len(caps.Builtins)) + } +} diff --git a/internal/capabilities/capabilities_test.go b/internal/capabilities/capabilities_test.go index b525a9c8..836ca163 100644 --- a/internal/capabilities/capabilities_test.go +++ b/internal/capabilities/capabilities_test.go @@ -33,25 +33,6 @@ func TestLookupFromFile(t *testing.T) { } } -func TestLookupFromURL(t *testing.T) { - t.Parallel() - - // Test that we can load a one of the existing OPA capabilities files - // via GitHub. - - caps, err := Lookup( - context.Background(), - "https://raw.githubusercontent.com/open-policy-agent/opa/main/capabilities/v0.55.0.json", - ) - if err != nil { - t.Errorf("unexpected error from Lookup: %v", err) - } - - if len(caps.Builtins) != 193 { - t.Errorf("OPA v0.55.0 capabilities should have 193 builtins, not %d", len(caps.Builtins)) - } -} - func TestLookupFromEmbedded(t *testing.T) { t.Parallel()