add support for being passed policy configurations (#113)

* add support for being passed policy configurations This adds support for providing an alternative policy definition. Signed-off-by: Mike Mason <[email protected]> * wait for ack before publishing changes This ensures we are listening before an ack event is published. Signed-off-by: Mike Mason <[email protected]> * update chart to accept spicedb.policySecretName Signed-off-by: Mike Mason <[email protected]> * extend golangci-lint timeout to 5 minutes Signed-off-by: Mike Mason <[email protected]> * +1 line Co-authored-by: E Camden Fisher <[email protected]> Signed-off-by: Mike Mason <[email protected]> --------- Signed-off-by: Mike Mason <[email protected]> Signed-off-by: Mike Mason <[email protected]> Co-authored-by: E Camden Fisher <[email protected]>
infratographer · Jun 19, 2023 · 3b71114 · 3b71114
1 parent 26f7382
commit 3b71114
Show file tree

Hide file tree

Showing 16 changed files with 254 additions and 37 deletions.
diff --git a/Makefile b/Makefile
@@ -65,7 +65,7 @@ lint: golint  ## Runs all lint checks.
 
 golint: | vendor $(TOOLS_DIR)/golangci-lint  ## Runs Go lint checks.
 	@echo Linting Go files...
-	@$(TOOLS_DIR)/golangci-lint run
+	@$(TOOLS_DIR)/golangci-lint run --timeout 5m
 
 clean:  ## Cleans generated files.
 	@echo Cleaning...

diff --git a/chart/permissions-api/templates/_helpers.tpl b/chart/permissions-api/templates/_helpers.tpl
@@ -3,3 +3,35 @@
 {{- define "permapi.listenPort" }}
 {{- .Values.config.server.port | default 8080 }}
 {{- end }}
+
+{{- define "permapi.volumes" }}
+{{- if or .Values.config.spicedb.caSecretName .Values.config.spicedb.policySecretName }}
+{{- with .Values.config.spicedb.caSecretName }}
+- name: spicedb-ca
+  secret:
+    secretName: {{ . }}
+{{- end }}
+{{- with .Values.config.spicedb.policySecretName }}
+- name: policy-file
+  secret:
+    secretName: {{ . }}
+{{- end }}
+{{- else -}}
+[]
+{{- end }}
+{{- end }}
+
+{{- define "permapi.volumeMounts" }}
+{{- if or .Values.config.spicedb.caSecretName .Values.config.spicedb.policySecretName }}
+{{- if .Values.config.spicedb.caSecretName }}
+- name: spicedb-ca
+  mountPath: /etc/ssl/spicedb/
+{{- end }}
+{{- if .Values.config.spicedb.policySecretName }}
+- name: policy-file
+  mountPath: /policy/policy.yaml
+{{- end }}
+{{- else -}}
+[]
+{{- end }}
+{{- end }}
diff --git a/chart/permissions-api/templates/deployment-server.yaml b/chart/permissions-api/templates/deployment-server.yaml
@@ -76,6 +76,10 @@ spec:
               value: "{{ .Values.config.spicedb.insecure }}"
             - name: PERMISSIONSAPI_SPICEDB_VERIFYCA
               value: "{{ .Values.config.spicedb.verifyCA }}"
+          {{- if .Values.config.spicedb.policySecretName }}
+            - name: PERMISSIONSAPI_SPICEDB_POLICYFILE
+              value: /policy/policy.yaml
+          {{- end }}
             - name: PERMISSIONSAPI_TRACING_ENABLED
               value: "{{ .Values.config.tracing.enabled }}"
             - name: PERMISSIONSAPI_TRACING_PROVIDER
@@ -121,10 +125,8 @@ spec:
           resources:
             {{- toYaml . | nindent 12 }}
           {{- end }}
+      volumeMounts: {{ include "permapi.volumeMounts" . | nindent 8 }}
       {{- with .Values.deployment.nodeSelector }}
-      volumeMounts:
-        - name: spicedb-ca
-          mountPath: /etc/ssl/spicedb/
       nodeSelector:
         {{- toYaml . | nindent 8 }}
       {{- end }}
@@ -136,9 +138,4 @@ spec:
       tolerations:
         {{- toYaml . | nindent 8 }}
       {{- end }}
-    {{- with .Values.config.spicedb.caSecretName }}  
-    volumes:
-      - name: spicedb-ca
-        secret:
-          secretName: {{ . }}
-    {{- end }}
+    volumes: {{ include "permapi.volumes" . | nindent 6 }}
diff --git a/chart/permissions-api/templates/deployment-worker.yaml b/chart/permissions-api/templates/deployment-worker.yaml
@@ -98,6 +98,10 @@ spec:
               value: "{{ .Values.config.spicedb.insecure }}"
             - name: PERMISSIONSAPI_SPICEDB_VERIFYCA
               value: "{{ .Values.config.spicedb.verifyCA }}"
+          {{- if .Values.config.spicedb.policySecretName }}
+            - name: PERMISSIONSAPI_SPICEDB_POLICYFILE
+              value: /policy/policy.yaml
+          {{- end }}
             - name: PERMISSIONSAPI_TRACING_ENABLED
               value: "{{ .Values.config.tracing.enabled }}"
             - name: PERMISSIONSAPI_TRACING_PROVIDER
@@ -143,10 +147,8 @@ spec:
           resources:
             {{- toYaml . | nindent 12 }}
           {{- end }}
+      volumeMounts: {{ include "permapi.volumeMounts" . | nindent 8 }}
       {{- with .Values.deployment.nodeSelector }}
-      volumeMounts:
-        - name: spicedb-ca
-          mountPath: /etc/ssl/spicedb/
       nodeSelector:
         {{- toYaml . | nindent 8 }}
       {{- end }}
@@ -158,9 +160,4 @@ spec:
       tolerations:
         {{- toYaml . | nindent 8 }}
       {{- end }}
-    {{- with .Values.config.spicedb.caSecretName }}  
-    volumes:
-      - name: spicedb-ca
-        secret:
-          secretName: {{ . }}
-    {{- end }}
+    volumes: {{ include "permapi.volumes" . | nindent 6 }}
diff --git a/chart/permissions-api/values.yaml b/chart/permissions-api/values.yaml
@@ -35,6 +35,8 @@ config:
     caSecretName: ""
     # pskSecretName is the name of the secret containing the pre-shared key for connecting to SpiceDB. This must contain a key, PERMISSIONSAPI_SPICEDB_KEY, that contains the PSK value
     pskSecretName: ""
+    # policySecretName is the name of the secret containing the policy file configuration
+    policySecretName: ""
 
   events:
     # url is the event server connection url

diff --git a/cmd/root.go b/cmd/root.go
@@ -58,6 +58,8 @@ func init() {
 	viperx.MustBindFlag(viper.GetViper(), "spicedb.verifyca", rootCmd.PersistentFlags().Lookup("spicedb-verifyca"))
 	rootCmd.PersistentFlags().String("spicedb-prefix", "", "spicedb prefix")
 	viperx.MustBindFlag(viper.GetViper(), "spicedb.prefix", rootCmd.PersistentFlags().Lookup("spicedb-prefix"))
+	rootCmd.PersistentFlags().String("spicedb-policy", "", "spicedb policy file")
+	viperx.MustBindFlag(viper.GetViper(), "spicedb.policyFile", rootCmd.PersistentFlags().Lookup("spicedb-policy"))
 
 	// Set up SIGINT/SIGTERM listener
 	sigCh = make(chan os.Signal, 1)

diff --git a/cmd/schema.go b/cmd/schema.go
@@ -9,6 +9,7 @@ import (
 	"go.infratographer.com/x/otelx"
 
 	"go.infratographer.com/permissions-api/internal/config"
+	"go.infratographer.com/permissions-api/internal/iapl"
 	"go.infratographer.com/permissions-api/internal/spicedbx"
 )
 
@@ -31,14 +32,37 @@ func init() {
 }
 
 func writeSchema(ctx context.Context, dryRun bool, cfg *config.AppConfig) {
-	schemaStr := spicedbx.GeneratedSchema("infratographer")
+	var (
+		err    error
+		policy iapl.Policy
+	)
+
+	if cfg.SpiceDB.PolicyFile != "" {
+		policy, err = iapl.NewPolicyFromFile(cfg.SpiceDB.PolicyFile)
+		if err != nil {
+			logger.Fatalw("unable to load new policy from schema file", "policy_file", cfg.SpiceDB.PolicyFile, "error", err)
+		}
+	} else {
+		logger.Warn("no spicedb policy file defined, using default policy")
+
+		policy = iapl.DefaultPolicy()
+	}
+
+	if err = policy.Validate(); err != nil {
+		logger.Fatalw("invalid spicedb policy", "error", err)
+	}
+
+	schemaStr, err := spicedbx.GenerateSchema("infratographer", policy.Schema())
+	if err != nil {
+		logger.Fatalw("failed to generate schema from policy", "error", err)
+	}
 
 	if dryRun {
 		fmt.Printf("%s", schemaStr)
 		return
 	}
 
-	err := otelx.InitTracer(cfg.Tracing, appName, logger)
+	err = otelx.InitTracer(cfg.Tracing, appName, logger)
 	if err != nil {
 		logger.Fatalw("unable to initialize tracing system", "error", err)
 	}

diff --git a/cmd/server.go b/cmd/server.go
@@ -13,6 +13,7 @@ import (
 
 	"go.infratographer.com/permissions-api/internal/api"
 	"go.infratographer.com/permissions-api/internal/config"
+	"go.infratographer.com/permissions-api/internal/iapl"
 	"go.infratographer.com/permissions-api/internal/query"
 	"go.infratographer.com/permissions-api/internal/spicedbx"
 )
@@ -50,7 +51,24 @@ func serve(ctx context.Context, cfg *config.AppConfig) {
 		logger.Fatalw("unable to initialize spicedb client", "error", err)
 	}
 
-	engine := query.NewEngine("infratographer", spiceClient)
+	var policy iapl.Policy
+
+	if cfg.SpiceDB.PolicyFile != "" {
+		policy, err = iapl.NewPolicyFromFile(cfg.SpiceDB.PolicyFile)
+		if err != nil {
+			logger.Fatalw("unable to load new policy from schema file", "policy_file", cfg.SpiceDB.PolicyFile, "error", err)
+		}
+	} else {
+		logger.Warn("no spicedb policy file defined, using default policy")
+
+		policy = iapl.DefaultPolicy()
+	}
+
+	if err = policy.Validate(); err != nil {
+		logger.Fatalw("invalid spicedb policy", "error", err)
+	}
+
+	engine := query.NewEngine("infratographer", spiceClient, query.WithPolicy(policy))
 
 	srv, err := echox.NewServer(
 		logger.Desugar(),

diff --git a/cmd/worker.go b/cmd/worker.go
@@ -10,6 +10,7 @@ import (
 	"go.infratographer.com/x/viperx"
 
 	"go.infratographer.com/permissions-api/internal/config"
+	"go.infratographer.com/permissions-api/internal/iapl"
 	"go.infratographer.com/permissions-api/internal/pubsub"
 	"go.infratographer.com/permissions-api/internal/query"
 	"go.infratographer.com/permissions-api/internal/spicedbx"
@@ -44,7 +45,24 @@ func worker(ctx context.Context, cfg *config.AppConfig) {
 		logger.Fatalw("unable to initialize spicedb client", "error", err)
 	}
 
-	engine := query.NewEngine("infratographer", spiceClient)
+	var policy iapl.Policy
+
+	if cfg.SpiceDB.PolicyFile != "" {
+		policy, err = iapl.NewPolicyFromFile(cfg.SpiceDB.PolicyFile)
+		if err != nil {
+			logger.Fatalw("unable to load new policy from schema file", "policy_file", cfg.SpiceDB.PolicyFile, "error", err)
+		}
+	} else {
+		logger.Warn("no spicedb policy file defined, using default policy")
+
+		policy = iapl.DefaultPolicy()
+	}
+
+	if err = policy.Validate(); err != nil {
+		logger.Fatalw("invalid spicedb policy", "error", err)
+	}
+
+	engine := query.NewEngine("infratographer", spiceClient, query.WithPolicy(policy))
 
 	subscriber, err := pubsub.NewSubscriber(ctx, cfg.Events.Subscriber, engine)
 	if err != nil {

diff --git a/go.mod b/go.mod
@@ -18,6 +18,7 @@ require (
 	go.opentelemetry.io/otel/trace v1.16.0
 	go.uber.org/zap v1.24.0
 	google.golang.org/grpc v1.55.0
+	gopkg.in/yaml.v3 v3.0.1
 )
 
 require (
@@ -100,5 +101,4 @@ require (
 	google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1 // indirect
 	google.golang.org/protobuf v1.30.0 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect
-	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
diff --git a/internal/iapl/policy.go b/internal/iapl/policy.go
@@ -2,8 +2,10 @@ package iapl
 
 import (
 	"fmt"
+	"os"
 
 	"go.infratographer.com/permissions-api/internal/types"
+	"gopkg.in/yaml.v3"
 )
 
 // PolicyDocument represents a partial authorization policy.
@@ -108,6 +110,22 @@ func NewPolicy(p PolicyDocument) Policy {
 	return &out
 }
 
+// NewPolicyFromFile reads the provided file path and returns a new Policy.
+func NewPolicyFromFile(filePath string) (Policy, error) {
+	file, err := os.Open(filePath)
+	if err != nil {
+		return nil, err
+	}
+
+	var policy PolicyDocument
+
+	if err := yaml.NewDecoder(file).Decode(&policy); err != nil {
+		return nil, err
+	}
+
+	return NewPolicy(policy), nil
+}
+
 func (v *policy) validateUnions() error {
 	for _, union := range v.p.Unions {
 		if _, ok := v.rt[union.Name]; ok {

diff --git a/internal/pubsub/subscriber_test.go b/internal/pubsub/subscriber_test.go
@@ -250,12 +250,17 @@ func TestNATS(t *testing.T) {
 
 		require.NoError(t, err)
 
+		ackErr := make(chan error, 1)
+
+		go func() {
+			ackErr <- nats.WaitForAck(consumerName, 5*time.Second)
+		}()
+
 		err = pub.PublishChange(ctx, input.subject, input.changeMessage)
 
 		require.NoError(t, err)
 
-		err = nats.WaitForAck(consumerName, time.Second)
-		if err != nil {
+		if err = <-ackErr; err != nil {
 			return testingx.TestResult[*Subscriber]{
 				Err: err,
 			}

diff --git a/internal/query/service.go b/internal/query/service.go
@@ -42,20 +42,21 @@ func (e *engine) cacheSchemaPrefixes() {
 
 // NewEngine returns a new client for making permissions queries.
 func NewEngine(namespace string, client *authzed.Client, options ...Option) Engine {
-	policy := iapl.DefaultPolicy()
-
 	e := &engine{
 		logger:    zap.NewNop().Sugar(),
 		namespace: namespace,
 		client:    client,
-		schema:    policy.Schema(),
 	}
 
 	for _, fn := range options {
 		fn(e)
 	}
 
-	e.cacheSchemaPrefixes()
+	if e.schema == nil {
+		e.schema = iapl.DefaultPolicy().Schema()
+
+		e.cacheSchemaPrefixes()
+	}
 
 	return e
 }
@@ -69,3 +70,12 @@ func WithLogger(logger *zap.SugaredLogger) Option {
 		e.logger = logger
 	}
 }
+
+// WithPolicy sets the policy for the engine
+func WithPolicy(policy iapl.Policy) Option {
+	return func(e *engine) {
+		e.schema = policy.Schema()
+
+		e.cacheSchemaPrefixes()
+	}
+}
diff --git a/internal/spicedbx/client.go b/internal/spicedbx/client.go
@@ -14,11 +14,12 @@ import (
 
 // Config values for a SpiceDB connection
 type Config struct {
-	Endpoint string
-	Key      string
-	Insecure bool
-	VerifyCA bool `mapstruct:"verifyca"`
-	Prefix   string
+	Endpoint   string
+	Key        string
+	Insecure   bool
+	VerifyCA   bool `mapstruct:"verifyca"`
+	Prefix     string
+	PolicyFile string
 }
 
 // NewClient returns a new spicedb/authzed client

diff --git a/permissions-api.example.yaml b/permissions-api.example.yaml
@@ -1,4 +1,3 @@
 oidc:
-  issuer: http://localhost:8081/default
+  issuer: http://mock-oauth2-server:8081/default
   audience: permissions-api
-  jwksuri: http://mock-oauth2-server:8081/default/jwks