0.7.0 Type check issue

[shwetambry]

Description

Observed Behavior: In versions 0.6.+ and 0.7.0, getting the type check error --
failed to validate node "rttest": failed to type-check template expression "has(schema.spec.dummytest) ? schema.spec.dummytest.dummya : dummyRef.spec[0].fieldblist" at path "spec.references": ERROR: :1:28: found no matching overload for '?:_' applied to '(bool, list(__type_schema.spec.dummytest.dummya.@idx), list(__type_dummyRef.spec.@idx.fieldblist.@idx))'

In kro version 0.5.1, it works correctly.

Reproduction Steps :
Here is my test RGD

apiVersion: kro.run/v1alpha1
kind: ResourceGraphDefinition
metadata:
  name: test-dummy
spec:
  schema:
    apiVersion: v1
    kind: TestApp
    spec:
      name: string
      dummytest:
        dummya: '[]Custom'
    types:
      Custom:
        subfieldstr: string
        subfieldint: integer
  resources:
  - id: dummyRef
    externalRef:
      apiVersion: kro.bug/v1
      kind: Dummy
      metadata:
        name: dummycr
        namespace: test
  - id: rttest
    template:
      apiVersion: kro.bug/v1
      kind: Resourcetemplate
      metadata:
        name: test-rt
        namespace: test
      spec:
        references: "${has(schema.spec.dummytest) ? 
          schema.spec.dummytest.dummya : dummyRef.spec[0].fieldblist}"
        

Here are the test CRDs

apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  name: dummies.kro.bug
spec:
  conversion:
    strategy: None
  group: kro.bug
  names:
    kind: Dummy
    listKind: DummyList
    plural: dummies
    shortNames:
    - dummy
    singular: dummy
  scope: Namespaced
  versions:
  - name: v1
    schema:
      openAPIV3Schema:
        properties:
          spec:
            items:
              properties:
                name: 
                  description: Name
                  type: string
                fieldaint:
                  description: Field a
                  type: integer
                fieldblist:
                  description: Field b
                  items:
                    properties:
                      subfieldstr:
                        description: Subfield 1
                        type: string
                      subfieldint:
                        description: Subfield 2
                        type: integer
                    required:
                    - subfieldstr
                    - subfieldint
                    type: object
                  type: array
              required:
              - name
              - fieldaint
              - fieldblist
              type: object
            type: array
        type: object
    served: true
    storage: true
    subresources:
      status: {}
---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  name: resourcetemplates.kro.bug
spec:
  conversion:
    strategy: None
  group: kro.bug
  names:
    kind: Resourcetemplate
    listKind: ResourcetemplateList
    plural: resourcetemplates
    shortNames:
    - resourcetemplate
    singular: resourcetemplate
  scope: Namespaced
  versions:
  - name: v1
    schema:
      openAPIV3Schema:
        properties:
          spec:
            properties:
              references:
                items:
                  properties:
                    subfieldstr:
                      description: Subfield 1
                      type: string
                    subfieldint:
                      description: Subfield 2
                      type: integer
                  required:
                  - subfieldstr
                  - subfieldint
                  type: object
                type: array
            type: object
        type: object
    served: true
    storage: true
    subresources:
      status: {}

Here are my test resources

apiVersion: kro.bug/v1
kind: Dummy
metadata:
  name: dummycr
  namespace: test
spec:
  - name: item1
    fieldaint: 1
    fieldblist:
      - subfieldstr: "First entry"
        subfieldint: 1
      - subfieldstr: "Second entry"
        subfieldint: 2
---
apiVersion: kro.run/v1
kind: TestApp
metadata:
  name: testapp-cr
  namespace: test
spec:
  name: "Test Application CR"

Using v0.5.1, the above TestApp resource successfully creates a Resourcetemplate resource, however the RGD breaks in 0.6.3 as well as 0.7.0.

Versions:

• kro version: 0.7.0, 0.6.+
• Kubernetes Version (kubectl version): 1.33

Involved Controllers:

• Controller URLs and Versions (if applicable):

Error Logs (if applicable)**:

failed to validate node "rttest": failed to type-check template expression "has(schema.spec.dummytest) ? schema.spec.dummytest.dummya : dummyRef.spec[0].fieldblist" at path "spec.references": ERROR: <input>:1:28: found no matching overload for '_?_:_' applied to '(bool, list(__type_schema.spec.dummytest.dummya.@idx), list(__type_dummyRef.spec.@idx.fieldblist.@idx))'

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

Which option describes the most your issue?

ResourceGraphDefinition (Create, Update, Deletion)

[jakobmoellerdev]

thanks for the report! do you mind running a retest on 0.7.1 for us?

[shwetambry]

It is the same error with version 0.7.1

[a-hilaly]

@shwetambry This is a mix of how kro assigns type names to schemas and how CEL does type checking. The ternary operator requires both branches to return the exact same type. So two lists or objects with identical fields but different schema paths are seen as incompatible. This worked in v0.5.1 because we weren't doing strict type checking back then.

For now you can work around it with something like:

has(schema.spec.dummytest) ? dyn(schema.spec.dummytest.dummya) : dyn(dummyRef.spec[0].fieldblist)`

[Mujib-Ahasan]

/assign

[Mujib-Ahasan]

@a-hilaly, I understand now that fails because the two branches have different inferred types and ternary requires an exact match. I wondering would it make sense for KRO to treat two different branches values as same type when their structures match, so users don’t need to add dyn() explicitly? Am I thinking in direction??

[Mujib-Ahasan]

Below is the code that is actually making noise, expression is passed in parseAndCheckCELExpression().

kro/pkg/graph/builder.go

Lines 827 to 857 in 4ec1551

	func validateTemplateExpressions(env *cel.Env, resource *Resource, typeProvider *krocel.DeclTypeProvider) error {
	for _, templateVariable := range resource.variables {
	if len(templateVariable.Expressions) == 1 {
	// Single expression - validate against expected types
	expression := templateVariable.Expressions[0]
	checkedAST, err := parseAndCheckCELExpression(env, expression)
	if err != nil {
	return fmt.Errorf("failed to type-check template expression %q at path %q: %w", expression, templateVariable.Path, err)
	}
	outputType := checkedAST.OutputType()
	if err := validateExpressionType(outputType, templateVariable.ExpectedType, expression, resource.id, templateVariable.Path, typeProvider); err != nil {
	return err
	}
	} else if len(templateVariable.Expressions) > 1 {
	// Multiple expressions - all must be strings for concatenation
	for _, expression := range templateVariable.Expressions {
	checkedAST, err := parseAndCheckCELExpression(env, expression)
	if err != nil {
	return fmt.Errorf("failed to type-check template expression %q at path %q: %w", expression, templateVariable.Path, err)
	}
	outputType := checkedAST.OutputType()
	if err := validateExpressionType(outputType, templateVariable.ExpectedType, expression, resource.id, templateVariable.Path, typeProvider); err != nil {
	return err
	}
	}
	}
	}
	return nil
	}

and there env.Check() is giving us the actual error. It would be a good Idea if we unify both types before the check.

kro/pkg/graph/builder.go

Lines 890 to 902 in 4ec1551

	func parseAndCheckCELExpression(env *cel.Env, expression string) (*cel.Ast, error) {
	parsedAST, issues := env.Parse(expression)
	if issues != nil && issues.Err() != nil {
	return nil, issues.Err()
	}
	checkedAST, issues := env.Check(parsedAST)
	if issues != nil && issues.Err() != nil {
	return nil, issues.Err()
	}
	return checkedAST, nil
	}

WDYT?? Any suggestion!?