Skip to content
Draft
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
46 changes: 34 additions & 12 deletions pkg/graph/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,16 +202,14 @@ func (b *Builder) NewResourceGraphDefinition(originalCR *v1alpha1.ResourceGraphD
}
}

// include the instance spec schema in the context as "schema". This will let us
// validate expressions such as ${schema.spec.someField}.
//
// not that we only include the spec and metadata fields, instance status references
// are not allowed in RGDs (yet)
schemaWithoutStatus, err := getSchemaWithoutStatus(instance.crd)
// include the instance schema in the context as "schema". This will let us
// validate expressions such as ${schema.spec.someField}, ${schema.metadata.name},
// and ${schema.status.someField}.
instanceSchema, err := getInstanceSchema(instance.crd)
if err != nil {
return nil, fmt.Errorf("failed to get schema without status: %w", err)
return nil, fmt.Errorf("failed to get instance schema: %w", err)
}
schemas["schema"] = schemaWithoutStatus
schemas["schema"] = instanceSchema

// First, build the dependency graph by inspecting CEL expressions.
// This extracts all resource dependencies and validates that:
Expand Down Expand Up @@ -831,8 +829,28 @@ func validateReadyWhenExpressions(env *cel.Env, resource *Resource) error {
return nil
}

// getSchemaWithoutStatus returns a schema from the CRD with the status field removed.
func getSchemaWithoutStatus(crd *extv1.CustomResourceDefinition) (*spec.Schema, error) {
// unrequireStatus modifies the schema to ensure "status" is not in the required fields list.
// This prevents CEL type-checking from treating status as a required field, which would incorrectly
// fail validation when status is legitimately absent (e.g., resources that haven't reconciled yet).
func unrequireStatus(s *spec.Schema) {
if s == nil || len(s.Required) == 0 {
return
}

// Remove "status" from required fields if present
required := make([]string, 0, len(s.Required))
for _, field := range s.Required {
if field != "status" {
required = append(required, field)
}
}
s.Required = required
Comment on lines +840 to +847
Copy link
Contributor

Choose a reason for hiding this comment

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

Would we gain readability on the intent using slices.DeleteFunc

Suggested change
// Remove "status" from required fields if present
required := make([]string, 0, len(s.Required))
for _, field := range s.Required {
if field != "status" {
required = append(required, field)
}
}
s.Required = required
s.Required = slices.DeleteFunc(s.Required, func(e string){
return e == "status"
})

}

// getInstanceSchema returns a schema from the CRD including spec, status, and metadata fields.
// This schema is used for CEL expression validation, allowing references to instance fields like
// ${schema.spec.field}, ${schema.status.field}, and ${schema.metadata.name}.
func getInstanceSchema(crd *extv1.CustomResourceDefinition) (*spec.Schema, error) {
crdCopy := crd.DeepCopy()

// TODO(a-hilaly) expand this function when we start support CRD upgrades.
Expand All @@ -846,8 +864,6 @@ func getSchemaWithoutStatus(crd *extv1.CustomResourceDefinition) (*spec.Schema,
openAPISchema.Properties = make(map[string]extv1.JSONSchemaProps)
}

delete(openAPISchema.Properties, "status")

specSchema, err := schema.ConvertJSONSchemaPropsToSpecSchema(openAPISchema)
if err != nil {
return nil, err
Expand All @@ -857,5 +873,11 @@ func getSchemaWithoutStatus(crd *extv1.CustomResourceDefinition) (*spec.Schema,
specSchema.Properties = make(map[string]spec.Schema)
}
specSchema.Properties["metadata"] = schema.ObjectMetaSchema

// Unrequire status before type-checking. This allows CEL expressions
// in dependent resources to reference instance status (e.g., ${schema.status.endpoint})
// without validation errors when the instance status hasn't been populated yet.
unrequireStatus(specSchema)

return specSchema, nil
}
157 changes: 157 additions & 0 deletions pkg/graph/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1766,6 +1766,163 @@ func TestGraphBuilder_CELTypeChecking(t *testing.T) {
},
wantErr: false,
},
{
name: "resource references instance status field (string)",
resourceGraphDefinitionOpts: []generator.ResourceGraphDefinitionOption{
generator.WithSchema(
"Application", "v1alpha1",
map[string]interface{}{
"region": "string",
},
map[string]interface{}{
"vpcID": "${vpc.status.vpcID}",
Copy link
Contributor

Choose a reason for hiding this comment

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

We also need tests to ensure that:

  1. when the status field can't be resolved (i.e. vpc.status == nil) the instance can be reconciled without error
  2. when the status field exists (i.e. vpc.status.vpcID = "vpc-xzy"), after reconcile, the instance has status status.vpcID: vpc-xyz`

},
),
generator.WithResource("vpc", map[string]interface{}{
"apiVersion": "ec2.services.k8s.aws/v1alpha1",
"kind": "VPC",
"metadata": map[string]interface{}{
"name": "app-vpc",
},
"spec": map[string]interface{}{
"cidrBlocks": []interface{}{"10.0.0.0/16"},
},
}, nil, nil),
generator.WithResource("subnet", map[string]interface{}{
"apiVersion": "ec2.services.k8s.aws/v1alpha1",
"kind": "Subnet",
"metadata": map[string]interface{}{
"name": "app-subnet",
},
"spec": map[string]interface{}{
"cidrBlock": "10.0.1.0/24",
"vpcID": "${schema.status.vpcID}",
},
}, nil, nil),
},
wantErr: false,
},
{
name: "resource references instance status field (nested object)",
resourceGraphDefinitionOpts: []generator.ResourceGraphDefinitionOption{
generator.WithSchema(
"NetworkApp", "v1alpha1",
map[string]interface{}{
"vpcName": "string",
},
map[string]interface{}{
"vpcState": "${vpc.status.state}",
},
),
generator.WithResource("vpc", map[string]interface{}{
"apiVersion": "ec2.services.k8s.aws/v1alpha1",
"kind": "VPC",
"metadata": map[string]interface{}{
"name": "${schema.spec.vpcName}",
},
"spec": map[string]interface{}{
"cidrBlocks": []interface{}{"10.0.0.0/16"},
},
}, nil, nil),
generator.WithResource("securitygroup", map[string]interface{}{
"apiVersion": "ec2.services.k8s.aws/v1alpha1",
"kind": "SecurityGroup",
"metadata": map[string]interface{}{
"name": "${schema.spec.vpcName}-sg",
},
"spec": map[string]interface{}{
"description": "VPC state is: ${schema.status.vpcState}",
},
}, nil, nil),
},
wantErr: false,
},
{
name: "multiple resources reference same instance status field",
resourceGraphDefinitionOpts: []generator.ResourceGraphDefinitionOption{
generator.WithSchema(
"Application", "v1alpha1",
map[string]interface{}{
"region": "string",
},
map[string]interface{}{
"primaryVPCID": "${vpc.status.vpcID}",
},
),
generator.WithResource("vpc", map[string]interface{}{
"apiVersion": "ec2.services.k8s.aws/v1alpha1",
"kind": "VPC",
"metadata": map[string]interface{}{
"name": "primary-vpc",
},
"spec": map[string]interface{}{
"cidrBlocks": []interface{}{"10.0.0.0/16"},
},
}, nil, nil),
generator.WithResource("subnet1", map[string]interface{}{
"apiVersion": "ec2.services.k8s.aws/v1alpha1",
"kind": "Subnet",
"metadata": map[string]interface{}{
"name": "subnet-1",
},
"spec": map[string]interface{}{
"cidrBlock": "10.0.1.0/24",
"vpcID": "${schema.status.primaryVPCID}",
},
}, nil, nil),
generator.WithResource("subnet2", map[string]interface{}{
"apiVersion": "ec2.services.k8s.aws/v1alpha1",
"kind": "Subnet",
"metadata": map[string]interface{}{
"name": "subnet-2",
},
"spec": map[string]interface{}{
"cidrBlock": "10.0.2.0/24",
"vpcID": "${schema.status.primaryVPCID}",
},
}, nil, nil),
},
wantErr: false,
},
{
name: "resource references instance status field (boolean computed)",
resourceGraphDefinitionOpts: []generator.ResourceGraphDefinitionOption{
generator.WithSchema(
"Application", "v1alpha1",
map[string]interface{}{
"region": "string",
},
map[string]interface{}{
"vpcReady": "${vpc.status.state == 'available'}",
},
),
generator.WithResource("vpc", map[string]interface{}{
"apiVersion": "ec2.services.k8s.aws/v1alpha1",
"kind": "VPC",
"metadata": map[string]interface{}{
"name": "app-vpc",
},
"spec": map[string]interface{}{
"cidrBlocks": []interface{}{"10.0.0.0/16"},
},
}, nil, nil),
generator.WithResource("subnet", map[string]interface{}{
"apiVersion": "ec2.services.k8s.aws/v1alpha1",
"kind": "Subnet",
"metadata": map[string]interface{}{
"name": "conditional-subnet",
"labels": map[string]interface{}{
"vpc-ready": "${schema.status.vpcReady ? 'true' : 'false'}",
},
},
"spec": map[string]interface{}{
"cidrBlock": "10.0.1.0/24",
"vpcID": "${vpc.status.vpcID}",
},
}, nil, nil),
},
wantErr: false,
},
}

for _, tt := range tests {
Expand Down