Skip to content

Commit

Permalink
fix: rule readiness check should require at least one rule to be load…
Browse files Browse the repository at this point in the history
…ed (#1061)

With this change, Oathkeeper now reports as "not ready" on the health check if not at least one valid rule is loaded.
  • Loading branch information
zepatrik committed Feb 7, 2023
1 parent e29a26a commit daa2994
Show file tree
Hide file tree
Showing 46 changed files with 586 additions and 625 deletions.
10 changes: 7 additions & 3 deletions api/credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ import (

func TestCredentialsHandler(t *testing.T) {
conf := internal.NewConfigurationWithDefaults(configx.SkipValidation())
conf.SetForTest(t, configuration.MutatorIDTokenIsEnabled, true)
conf.SetForTest(t, configuration.MutatorIDTokenJWKSURL, "file://../test/stub/jwks-rsa-multiple.json")
conf.SetForTest(t, configuration.MutatorIDTokenIssuerURL, "https://example.com")
conf.SetForTest(t, configuration.AuthenticatorAnonymousIsEnabled, true)
conf.SetForTest(t, configuration.AuthorizerAllowIsEnabled, true)

r := internal.NewRegistry(conf)

Expand Down Expand Up @@ -57,9 +61,9 @@ func TestCredentialsHandler(t *testing.T) {

var j jose.JSONWebKeySet
require.NoError(t, json.NewDecoder(res.Body).Decode(&j))
assert.Len(t, j.Key("3e0edde4-12ad-425d-a783-135f46eac57e"), 1, "The public key should be broadcasted")
assert.Len(t, j.Key("81be3441-5303-4c52-b00d-bbdfadc75633"), 1, "The public key should be broadcasted")
assert.Len(t, j.Key("f4190122-ae96-4c29-8b79-56024e459d80"), 1, "The public key generated from the private key should be broadcasted")
require.Len(t, j.Key("3e0edde4-12ad-425d-a783-135f46eac57e"), 1, "The public key should be broadcasted")
require.Len(t, j.Key("81be3441-5303-4c52-b00d-bbdfadc75633"), 1, "The public key should be broadcasted")
require.Len(t, j.Key("f4190122-ae96-4c29-8b79-56024e459d80"), 1, "The public key generated from the private key should be broadcasted")
assert.IsType(t, new(rsa.PublicKey), j.Key("3e0edde4-12ad-425d-a783-135f46eac57e")[0].Key, "Ensure a public key")
assert.IsType(t, new(rsa.PublicKey), j.Key("f4190122-ae96-4c29-8b79-56024e459d80")[0].Key, "Ensure a public key")
assert.IsType(t, new(rsa.PublicKey), j.Key("81be3441-5303-4c52-b00d-bbdfadc75633")[0].Key, "Ensure a public key")
Expand Down
17 changes: 15 additions & 2 deletions api/health_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ import (
"testing"
"time"

"github.com/ory/herodot"
"github.com/ory/oathkeeper/driver/configuration"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/ory/oathkeeper/internal"
rulereadiness "github.com/ory/oathkeeper/rule/readiness"
"github.com/ory/oathkeeper/x"
)

Expand All @@ -33,6 +35,17 @@ type statusError struct {

func TestHealth(t *testing.T) {
conf := internal.NewConfigurationWithDefaults()
conf.SetForTest(t, configuration.AccessRuleRepositories, []string{"file://../test/stub/rules.json"})
conf.SetForTest(t, configuration.AuthorizerAllowIsEnabled, true)
conf.SetForTest(t, configuration.AuthorizerDenyIsEnabled, true)
conf.SetForTest(t, configuration.AuthenticatorNoopIsEnabled, true)
conf.SetForTest(t, configuration.AuthenticatorAnonymousIsEnabled, true)
conf.SetForTest(t, configuration.MutatorNoopIsEnabled, true)
conf.SetForTest(t, "mutators.header.config", map[string]interface{}{"headers": map[string]interface{}{}})
conf.SetForTest(t, configuration.MutatorHeaderIsEnabled, true)
conf.SetForTest(t, configuration.MutatorIDTokenJWKSURL, "https://stub/.well-known/jwks.json")
conf.SetForTest(t, configuration.MutatorIDTokenIssuerURL, "https://stub")
conf.SetForTest(t, configuration.MutatorIDTokenIsEnabled, true)
r := internal.NewRegistry(conf)

router := x.NewAPIRouter()
Expand All @@ -58,7 +71,7 @@ func TestHealth(t *testing.T) {
require.Equal(t, http.StatusServiceUnavailable, res.StatusCode)
require.NoError(t, json.NewDecoder(res.Body).Decode(&result))
assert.Empty(t, result.Status)
assert.Equal(t, rulereadiness.ErrRuleNotYetLoaded.Error(), result.Error.Message)
assert.Equal(t, herodot.ErrNotFound.ErrorField, result.Error.Message)

r.Init()
// Waiting for rule load and health event propagation
Expand Down
6 changes: 3 additions & 3 deletions cmd/health_alive.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import (
"fmt"
"os"

"github.com/spf13/cobra"
"github.com/ory/oathkeeper/internal/httpclient/client/health"

"github.com/ory/oathkeeper/internal/httpclient/client/api"
"github.com/spf13/cobra"

"github.com/ory/x/cmdx"
)
Expand All @@ -28,7 +28,7 @@ Note:
Run: func(cmd *cobra.Command, _ []string) {
client := newClient(cmd)

r, err := client.API.IsInstanceAlive(api.NewIsInstanceAliveParams())
r, err := client.Health.IsInstanceAlive(health.NewIsInstanceAliveParams())
// If err, print err and exit 1
cmdx.Must(err, "%s", err)
// Print payload
Expand Down
5 changes: 3 additions & 2 deletions cmd/health_ready.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ import (
"fmt"
"os"

"github.com/ory/oathkeeper/internal/httpclient/client/health"

"github.com/spf13/cobra"

"github.com/ory/oathkeeper/internal/httpclient/client/api"
"github.com/ory/x/cmdx"
)

Expand All @@ -27,7 +28,7 @@ Note:
Run: func(cmd *cobra.Command, args []string) {
client := newClient(cmd)

r, err := client.API.IsInstanceReady(api.NewIsInstanceReadyParams())
r, err := client.Health.IsInstanceReady(health.NewIsInstanceReadyParams())
// If err, print err and exit 1
cmdx.Must(err, "%s", err)
// Print payload
Expand Down
19 changes: 6 additions & 13 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ package cmd
import (
"fmt"
"os"
"runtime"

"github.com/pkg/errors"
"github.com/spf13/cobra"

"github.com/ory/x/cmdx"

_ "github.com/ory/jsonschema/v3/fileloader"
_ "github.com/ory/jsonschema/v3/httploader"
"github.com/ory/x/configx"
Expand All @@ -25,22 +27,13 @@ var RootCmd = &cobra.Command{
// This is called by main.main(). It only needs to happen once to the rootCmd.
func Execute() {
if err := RootCmd.Execute(); err != nil {
fmt.Println(err)
if !errors.Is(err, cmdx.ErrNoPrintButFail) {
fmt.Println(err)
}
os.Exit(-1)
}
}

func init() {
configx.RegisterFlags(RootCmd.PersistentFlags())
}

func userHomeDir() string {
if runtime.GOOS == "windows" {
home := os.Getenv("HOMEDRIVE") + os.Getenv("HOMEPATH")
if home == "" {
home = os.Getenv("USERPROFILE")
}
return home
}
return os.Getenv("HOME")
}
96 changes: 58 additions & 38 deletions cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,18 @@
package cmd

import (
"encoding/base64"
"fmt"
"net/http"
"os"
"testing"
"time"

"github.com/phayes/freeport"
"github.com/spf13/cobra"

"github.com/ory/x/cmdx"

"github.com/stretchr/testify/assert"
"github.com/phayes/freeport"
)

var apiPort, proxyPort int
Expand Down Expand Up @@ -48,7 +51,39 @@ func init() {
os.Setenv("AUTHENTICATORS_ANONYMOUS_ENABLED", "true")
os.Setenv("AUTHORIZERS_ALLOW_ENABLED", "true")
os.Setenv("MUTATORS_NOOP_ENABLED", "true")
os.Setenv("ACCESS_RULES_REPOSITORIES", "inline://W3siaWQiOiJ0ZXN0LXJ1bGUtNCIsInVwc3RyZWFtIjp7InByZXNlcnZlX2hvc3QiOnRydWUsInN0cmlwX3BhdGgiOiIvYXBpIiwidXJsIjoibXliYWNrZW5kLmNvbS9hcGkifSwibWF0Y2giOnsidXJsIjoibXlwcm94eS5jb20vYXBpIiwibWV0aG9kcyI6WyJHRVQiLCJQT1NUIl19LCJhdXRoZW50aWNhdG9ycyI6W3siaGFuZGxlciI6Im5vb3AifSx7ImhhbmRsZXIiOiJhbm9ueW1vdXMifV0sImF1dGhvcml6ZXIiOnsiaGFuZGxlciI6ImFsbG93In0sIm11dGF0b3JzIjpbeyJoYW5kbGVyIjoibm9vcCJ9XX1d")
os.Setenv("ACCESS_RULES_REPOSITORIES", "inline://"+base64.StdEncoding.EncodeToString([]byte(`[
{
"id": "test-rule-4",
"upstream": {
"preserve_host": true,
"strip_path": "/api",
"url": "https://mybackend.com/api"
},
"match": {
"url": "myproxy.com/api",
"methods": [
"GET",
"POST"
]
},
"authenticators": [
{
"handler": "noop"
},
{
"handler": "anonymous"
}
],
"authorizer": {
"handler": "allow"
},
"mutators": [
{
"handler": "noop"
}
]
}
]`)))
}

func ensureOpen(t *testing.T, port int) bool {
Expand All @@ -64,18 +99,28 @@ func ensureOpen(t *testing.T, port int) bool {
func TestCommandLineInterface(t *testing.T) {
var osArgs = make([]string, len(os.Args))
copy(osArgs, os.Args)
cmd := cmdx.CommandExecuter{
New: func() *cobra.Command {
cp := *RootCmd
return &cp
},
}

// start server, and wait for the ports to be open
cmd.ExecBackground(nil, os.Stdout, os.Stderr, "serve", "--disable-telemetry")
var count = 0
for ensureOpen(t, apiPort) && ensureOpen(t, proxyPort) {
t.Logf("Port is not yet open, retrying attempt #%d..", count)
count++
if count > 50 {
t.FailNow()
}
time.Sleep(100 * time.Millisecond)
}

for _, c := range []struct {
args []string
wait func() bool
expectErr bool
args []string
}{
{
args: []string{"serve", "--disable-telemetry"},
wait: func() bool {
return ensureOpen(t, apiPort) && ensureOpen(t, proxyPort)
},
},
{args: []string{"rules", fmt.Sprintf("--endpoint=http://127.0.0.1:%d/", apiPort), "list"}},
{args: []string{"rules", fmt.Sprintf("--endpoint=http://127.0.0.1:%d/", apiPort), "get", "test-rule-4"}},
{args: []string{"health", fmt.Sprintf("--endpoint=http://127.0.0.1:%d/", apiPort), "alive"}},
Expand All @@ -85,33 +130,8 @@ func TestCommandLineInterface(t *testing.T) {
{args: []string{"credentials", "generate", "--alg", "HS256"}},
{args: []string{"credentials", "generate", "--alg", "RS512"}},
} {
RootCmd.SetArgs(c.args)

t.Run(fmt.Sprintf("command=%v", c.args), func(t *testing.T) {
if c.wait != nil {
go func() {
assert.Nil(t, RootCmd.Execute())
}()
}

if c.wait != nil {
var count = 0
for c.wait() {
t.Logf("Port is not yet open, retrying attempt #%d..", count)
count++
if count > 5 {
t.FailNow()
}
time.Sleep(time.Second)
}
} else {
err := RootCmd.Execute()
if c.expectErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
}
cmd.ExecNoErr(t, c.args...)
})
}
}
8 changes: 6 additions & 2 deletions cmd/rules_get.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,17 @@ var getCmd = &cobra.Command{
oathkeeper rules --endpoint=http://localhost:4456/ get rule-1
`,
Run: func(cmd *cobra.Command, args []string) {
RunE: func(cmd *cobra.Command, args []string) error {
cmdx.ExactArgs(cmd, args, 1)
client := newClient(cmd)

r, err := client.API.GetRule(api.NewGetRuleParams().WithID(args[0]))
cmdx.Must(err, "%s", err)
if err != nil {
fmt.Fprintf(cmd.ErrOrStderr(), "Could not get rule: %s", err)
return cmdx.FailSilently(cmd)
}
fmt.Println(cmdx.FormatResponse(r.Payload))
return nil
},
}

Expand Down
1 change: 1 addition & 0 deletions driver/configuration/config_keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const (
MutatorHydratorIsEnabled Key = "mutators.hydrator.enabled"
MutatorIDTokenIsEnabled Key = "mutators.id_token.enabled"
MutatorIDTokenJWKSURL Key = "mutators.id_token.config.jwks_url"
MutatorIDTokenIssuerURL Key = "mutators.id_token.config.issuer_url"
)

// Authenticators
Expand Down
20 changes: 0 additions & 20 deletions driver/health/event_manager.go

This file was deleted.

Loading

0 comments on commit daa2994

Please sign in to comment.