Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
2 changes: 1 addition & 1 deletion constraint/pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ import (
"github.com/open-policy-agent/frameworks/constraint/pkg/handler"
"github.com/open-policy-agent/frameworks/constraint/pkg/instrumentation"
"github.com/open-policy-agent/frameworks/constraint/pkg/types"
admissionv1 "k8s.io/api/admission/v1"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
admissionv1 "k8s.io/api/admission/v1"
)

const statusField = "status"
Expand Down
8 changes: 8 additions & 0 deletions constraint/pkg/client/clienttest/cts/constraints.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,14 @@ func WantData(data string) ConstraintArg {
}
}

// WantEnvironment sets the expected namespace environment label.
// Only meaningful for CheckNamespace constraints.
func WantEnvironment(env string) ConstraintArg {
return func(u *unstructured.Unstructured) error {
return unstructured.SetNestedField(u.Object, env, "spec", "parameters", "wantEnvironment")
}
}

// EnforcementAction sets the action to be taken if the Constraint is violated.
func EnforcementAction(action string) ConstraintArg {
return func(u *unstructured.Unstructured) error {
Expand Down
59 changes: 59 additions & 0 deletions constraint/pkg/client/clienttest/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,3 +372,62 @@ func TemplateFuture() *templates.ConstraintTemplate {

return ct
}

const KindCheckNamespace = "CheckNamespace"

// moduleCheckNamespace defines a Rego package which checks the namespace object
// passed via input.review.namespaceObject. This tests that namespace data is available to
// Rego policies for namespace-based policy decisions.
const moduleCheckNamespace = `
package foo

violation[{"msg": msg}] {
# Check if namespace is provided and has the expected label
input.review.namespaceObject
ns := input.review.namespaceObject
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we need the guard if object doesn't have namespace?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added the check, if its nil then the variable results in undefined. So it should be alright, but added the check regardless.

not ns.metadata.labels.environment
msg := "namespace is missing environment label"
}

violation[{"msg": msg}] {
# Check if namespace has specific label value
input.review.namespaceObject
ns := input.review.namespaceObject
ns.metadata.labels.environment
ns.metadata.labels.environment != input.parameters.wantEnvironment
msg := sprintf("namespace has environment %v but want %v", [ns.metadata.labels.environment, input.parameters.wantEnvironment])
}
`

// TemplateCheckNamespace returns a ConstraintTemplate that validates namespace
// labels via input.review.namespaceObject. This tests the Rego driver's namespace support.
func TemplateCheckNamespace() *templates.ConstraintTemplate {
ct := &templates.ConstraintTemplate{}

ct.SetName("checknamespace")
ct.Spec.CRD.Spec.Names.Kind = KindCheckNamespace
ct.Spec.CRD.Spec.Validation = &templates.Validation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"wantEnvironment": {Type: "string"},
},
},
}

ct.Spec.Targets = []templates.Target{{
Target: handlertest.TargetName,
Code: []templates.Code{
{
Engine: schema.Name,
Source: &templates.Anything{
Value: (&schema.Source{
Rego: moduleCheckNamespace,
}).ToUnstructured(),
},
},
},
}}

return ct
}
5 changes: 5 additions & 0 deletions constraint/pkg/client/drivers/rego/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,11 @@ func (d *Driver) Query(ctx context.Context, target string, constraints []*unstru
opt(cfg)
}

// Add namespaceObject to review before the loop. This enables policies to access
// namespace labels and metadata via input.review.namespaceObject.
// Value is the namespace object for namespaced resources, or nil for cluster-scoped resources.
reviewMap["namespaceObject"] = cfg.Namespace

var statsEntries []*instrumentation.StatsEntry

for kind, kindConstraints := range constraintsByKind {
Expand Down
1 change: 1 addition & 0 deletions constraint/pkg/client/drivers/rego/rego.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ violation[response] {
key := input.constraints[_]
# Construct the input object from the Constraint and temporary object in storage.
# Silently exits if the Constraint no longer exists.
# Note: input.review already contains namespaceObject if available (set by driver).
inp := {
"review": input.review,
"parameters": data.constraints[key.kind][key.name],
Expand Down
190 changes: 190 additions & 0 deletions constraint/pkg/client/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1130,3 +1130,193 @@ func TestE2E_Client_GetDescriptionForStat(t *testing.T) {
}
}
}

// TestClient_Review_Namespace tests that namespace data is properly passed
// to the Rego driver via input.review.namespaceObject for namespace-based policy decisions.
func TestClient_Review_Namespace(t *testing.T) {
tests := []struct {
name string
namespace map[string]interface{}
wantEnv string
wantResults int
wantMsg string
}{
{
name: "no namespace provided - expects violation for missing namespace",
namespace: nil,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you add a case for empty namespace too

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added a test for this.

wantEnv: "production",
wantResults: 1,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we want this to be 0 or 1?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not really, the test policy is written so that it will result in violation when namespace is nil or empty.

wantMsg: "namespace is missing environment label",
},
{
name: "empty namespace object - expects violation for missing namespace",
namespace: map[string]interface{}{},
wantEnv: "production",
wantResults: 1,
wantMsg: "namespace is missing environment label",
},
{
name: "namespace with matching environment label",
namespace: map[string]interface{}{
"metadata": map[string]interface{}{
"name": "test-ns",
"labels": map[string]interface{}{
"environment": "production",
},
},
},
wantEnv: "production",
wantResults: 0, // No violation - environment matches
},
{
name: "namespace with wrong environment label",
namespace: map[string]interface{}{
"metadata": map[string]interface{}{
"name": "test-ns",
"labels": map[string]interface{}{
"environment": "staging",
},
},
},
wantEnv: "production",
wantResults: 1,
wantMsg: "namespace has environment staging but want production",
},
{
name: "namespace missing environment label",
namespace: map[string]interface{}{
"metadata": map[string]interface{}{
"name": "test-ns",
"labels": map[string]interface{}{
"team": "platform",
},
},
},
wantEnv: "production",
wantResults: 1,
wantMsg: "namespace is missing environment label",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := context.Background()

c := clienttest.New(t)

ct := clienttest.TemplateCheckNamespace()
_, err := c.AddTemplate(ctx, ct)
if err != nil {
t.Fatal(err)
}

constraint := cts.MakeConstraint(t, clienttest.KindCheckNamespace, "constraint", cts.WantEnvironment(tt.wantEnv))
_, err = c.AddConstraint(ctx, constraint)
if err != nil {
t.Fatal(err)
}

review := handlertest.NewReview("test-ns", "test-obj", "test-data")

// Pass namespace via reviews.Namespace option
var opts []reviews.ReviewOpt
if tt.namespace != nil {
opts = append(opts, reviews.Namespace(tt.namespace))
}

responses, err := c.Review(ctx, review, opts...)
if err != nil {
t.Fatal(err)
}

results := responses.Results()
if len(results) != tt.wantResults {
t.Errorf("got %d results, want %d. Results: %v", len(results), tt.wantResults, results)
}

if tt.wantResults > 0 && len(results) > 0 {
if results[0].Msg != tt.wantMsg {
t.Errorf("got message %q, want %q", results[0].Msg, tt.wantMsg)
}
}
})
}
}

// TestClient_Review_ClusterScopedResource verifies that cluster-scoped resources
// (resources without a namespace, like ClusterRole, PersistentVolume, etc.)
// can be reviewed correctly. This ensures empty namespace handling doesn't cause
// issues for cluster-scoped resources, including when namespace-aware policies are used.
func TestClient_Review_ClusterScopedResource(t *testing.T) {
t.Run("with deny policy", func(t *testing.T) {
ctx := context.Background()

c := clienttest.New(t)

// Use TemplateDeny which unconditionally denies - doesn't depend on namespace
ct := clienttest.TemplateDeny()
_, err := c.AddTemplate(ctx, ct)
if err != nil {
t.Fatal(err)
}

constraint := cts.MakeConstraint(t, clienttest.KindDeny, "deny-all")
_, err = c.AddConstraint(ctx, constraint)
if err != nil {
t.Fatal(err)
}

// Create a cluster-scoped review (empty namespace in the object itself)
review := handlertest.NewReview("", "cluster-resource", "test-data")

responses, err := c.Review(ctx, review)
if err != nil {
t.Fatalf("unexpected error during review: %v", err)
}

results := responses.Results()
if len(results) != 1 {
t.Errorf("got %d results, want 1. Results: %v", len(results), results)
}
})

t.Run("with namespace-aware policy", func(t *testing.T) {
ctx := context.Background()

c := clienttest.New(t)

// Use TemplateCheckNamespace which checks namespace labels
ct := clienttest.TemplateCheckNamespace()
_, err := c.AddTemplate(ctx, ct)
if err != nil {
t.Fatal(err)
}

constraint := cts.MakeConstraint(t, clienttest.KindCheckNamespace, "check-ns",
cts.WantEnvironment("production"))
_, err = c.AddConstraint(ctx, constraint)
if err != nil {
t.Fatal(err)
}

// Create a cluster-scoped review (empty namespace in the object)
review := handlertest.NewReview("", "cluster-resource", "test-data")

responses, err := c.Review(ctx, review)
if err != nil {
t.Fatalf("unexpected error during review: %v", err)
}

results := responses.Results()
if len(results) != 1 {
t.Errorf("got %d results, want 1. Results: %v", len(results), results)
}

if len(results) > 0 {
wantMsg := "namespace is missing environment label"
if results[0].Msg != wantMsg {
t.Errorf("got message %q, want %q", results[0].Msg, wantMsg)
}
}
})
}
13 changes: 13 additions & 0 deletions constraint/pkg/client/reviews/review_opts.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
package reviews

// ReviewCfg contains configuration options for a single review query.
type ReviewCfg struct {
TracingEnabled bool
StatsEnabled bool
EnforcementPoint string
// Namespace is the namespace object for the resource being reviewed.
// For namespaced resources, this contains the full namespace object
// including metadata and labels. For cluster-scoped resources, this is nil.
Namespace map[string]interface{}
}

// ReviewOpt specifies optional arguments for Query driver calls.
Expand Down Expand Up @@ -32,3 +37,11 @@ func EnforcementPoint(ep string) ReviewOpt {
cfg.EnforcementPoint = ep
}
}

// Namespace sets the namespace object for the review.
// This makes the namespace available to policy templates.
func Namespace(ns map[string]interface{}) ReviewOpt {
return func(cfg *ReviewCfg) {
cfg.Namespace = ns
}
}
2 changes: 1 addition & 1 deletion constraint/pkg/client/template_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ func makeMatchers(targets []handler.TargetHandler, constraint *unstructured.Unst
return result, nil
}

// MatchesOperation checks if the given operation type matches any of the template's target operations
// MatchesOperation checks if the given operation type matches any of the template's target operations.
func (e *templateClient) MatchesOperation(operation string) bool {
if len(e.template.Spec.Targets) != 1 {
// for backward compatibility, matching all templates by default
Expand Down
4 changes: 2 additions & 2 deletions constraint/pkg/client/template_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func TestTemplateClient_MatchesOperation_BackwardCompatibility(t *testing.T) {
}
}

// Benchmark test to ensure the function is performant
// Benchmark test to ensure the function is performant.
func BenchmarkTemplateClient_MatchesOperation(b *testing.B) {
tc := &templateClient{
template: &templates.ConstraintTemplate{
Expand All @@ -277,4 +277,4 @@ func BenchmarkTemplateClient_MatchesOperation(b *testing.B) {
for i := 0; i < b.N; i++ {
tc.MatchesOperation("UPDATE")
}
}
}
Loading