Skip to content
Merged
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
1 change: 1 addition & 0 deletions .github/workflows/pr-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ jobs:
uses: ansible/ansible-lint@main
with:
working_directory: "roles/"
setup_python: "false"

build-operator-bundle:
needs: detect-changes
Expand Down
47 changes: 46 additions & 1 deletion roles/tackle/templates/customresource-schema.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,29 @@ spec:
- name
title: Coordinates
type: object
- definition:
'$schema': https://json-schema.org/draft/2020-12/schema
properties:
organization:
description: Organization name.
minLength: 1
type: string
space:
description: Space name.
minLength: 1
type: string
name:
description: Application name.
minLength: 1
type: string
required:
- organization
- space
- name
title: Coordinates
type: object
migration: >
.organization = "unknown"
---
kind: Schema
apiVersion: tackle.konveyor.io/v1alpha1
Expand Down Expand Up @@ -61,4 +84,26 @@ spec:
minItems: 1
required:
- spaces

- definition:
$schema: https://json-schema.org/draft/2020-12/schema
title: Coordinates
type: object
properties:
organizations:
description: Organization names.
type: array
items:
type: string
minLength: 1
spaces:
description: Space names.
type: array
items:
type: string
minLength: 1
names:
description: Application names. Each may be a glob expression.
type: array
items:
type: string
minLength: 1
Comment on lines +87 to +109
Copy link

@coderabbitai coderabbitai bot Nov 4, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Critical: Schema structure doesn't match the described nested hierarchy.

The PR description states this adds support for "organizations with nested spaces and applications" (e.g., Organization "A" has space "Zoo" with applications "Lion" and "Tiger"). However, the current implementation defines three flat, parallel arrays (organizations, spaces, names) with no structural relationship between them.

This flat structure cannot express:

  • Which spaces belong to which organizations
  • Which applications belong to which spaces
  • The hierarchical filtering described in the PR objectives

Based on the past review comments and PR description, the schema should use a nested structure where:

  • Each organization object has a name and optional spaces array
  • Each space object has a name and optional applications array

Additionally, no required fields are specified in this schema version. Based on your earlier comment ("Only the root 'organizations' and 'name' fields should be required"), please add:

+        required:
+          - organizations

However, since "name" is now inside the names array (not a root field), please clarify whether you meant names should be required at the root level, or if the current flat structure is intentional despite the nested description in the PR objectives.

Would you like me to generate a nested schema structure that matches the PR description, or is the flat structure intentional?


I'll verify this review comment by examining the actual file and its context.


🏁 Script executed:

#!/bin/bash
# Find and display the customresource-schema file with context around lines 87-109
fd -e j2 -e yml 'customresource-schema' --exec cat {} \;

Length of output: 2707


🏁 Script executed:

#!/bin/bash
# Search for schema or nested structure definitions in the file
fd -e j2 -e yml 'customresource-schema' --exec head -120 {} \;

Length of output: 2707


🏁 Script executed:

#!/bin/bash
# Look for any references to "nested", "organizations", "spaces" in the repository
rg -n "organizations|nested.*spaces|spaces.*applications" --type yaml --type txt -C2

Length of output: 3070


🏁 Script executed:

#!/bin/bash
# Check for PR-related files or documentation
fd -e md -e txt 'README|CHANGELOG|PR|description' | head -20

Length of output: 24


🏁 Script executed:

#!/bin/bash
# Get git log to find PR description or commit messages
git log --oneline --all -20 | head -20

Length of output: 1012


🏁 Script executed:

#!/bin/bash
# Search for any schema definitions with "required" fields
rg -n "required:" --type yaml -A2 -B2

Length of output: 62501


🏁 Script executed:

#!/bin/bash
# Look for schema definitions mentioning "Coordinates"
rg -n "Coordinates|schema.*object" --type yaml -A10 -B2

Length of output: 1583


🏁 Script executed:

#!/bin/bash
# Check full file structure
fd 'customresource-schema.yml.j2' --exec wc -l {} \;

Length of output: 57


🏁 Script executed:

#!/bin/bash
# Display lines around 87-109 with more context
fd 'customresource-schema.yml.j2' --exec sed -n '70,120p' {} \;

Length of output: 1103


🏁 Script executed:

#!/bin/bash
# Look for nested structure patterns in yaml templates
rg -n "properties:" --type yaml -A15 | head -80

Length of output: 7500


🏁 Script executed:

#!/bin/bash
# Check for array items with nested properties
rg -n "items:" --type yaml -A5 -B2

Length of output: 72989


🏁 Script executed:

#!/bin/bash
# Search for "Definition" or filter definitions
rg -n "definition:|Coordinates" --type yaml -B5 -A25

Length of output: 56944


🏁 Script executed:

#!/bin/bash
# Get complete context of the schema around lines 87-109
fd 'customresource-schema.yml.j2' --exec sed -n '1,130p' {} \;

Length of output: 2707


🏁 Script executed:

#!/bin/bash
# Look for required fields specification
rg -n "^\s+required:" --type yaml -B5 -A3

Length of output: 117584


🏁 Script executed:

#!/bin/bash
# Check git diff to see what changed in this PR
git diff HEAD^ HEAD -- roles/tackle/templates/customresource-schema.yml.j2 | head -200

Length of output: 2165


🏁 Script executed:

#!/bin/bash
# Look for current branch info
git branch -v

Length of output: 122


🏁 Script executed:

#!/bin/bash
# Check PR body or description
git log -1 --format="%B" | head -50

Length of output: 58


🏁 Script executed:

#!/bin/bash
# Search for any example or test files showing expected structure
fd -e json -e yaml 'test|example|spec' | xargs grep -l "organizations\|coordinates" 2>/dev/null | head -5

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Look for filtering logic that might indicate expected structure
rg -n "filter|Filter" --type go --type py --type ts -B2 -A5 | head -100

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Check if there's documentation about the schema
fd -e adoc -e md 'schema\|api\|definition' --exec head -50 {} \;

Length of output: 0


🏁 Script executed:

#!/bin/bash
# List all files in the tackle templates directory to understand context
fd '' roles/tackle/templates/ --type f | head -30

Length of output: 1575


🏁 Script executed:

#!/bin/bash
# Display the complete customresource-schema.yml.j2 file
cat roles/tackle/templates/customresource-schema.yml.j2

Length of output: 2707


🏁 Script executed:

#!/bin/bash
# Look for issue or PR references in commit messages
git log --all --grep="nested\|Coordinates\|organization" --oneline | head -10

Length of output: 165


🏁 Script executed:

#!/bin/bash
# Check for any comments or documentation strings in the schema file
grep -n "#\|description\|title" roles/tackle/templates/customresource-schema.yml.j2 | head -50

Length of output: 611


🏁 Script executed:

#!/bin/bash
# Look at git history of this specific section
git log -p --follow -- roles/tackle/templates/customresource-schema.yml.j2 | head -300

Length of output: 59


🏁 Script executed:

#!/bin/bash
# Search in entire repo for references to nested organizations/spaces/applications structure
rg -n "organization.*space.*application|space.*application|nested" -C3 | head -100

Length of output: 404


🏁 Script executed:

#!/bin/bash
# Get current file state directly
if [ -f roles/tackle/templates/customresource-schema.yml.j2 ]; then
  echo "=== File exists ===" && wc -l roles/tackle/templates/customresource-schema.yml.j2
  echo "=== Lines 1-50 ===" && sed -n '1,50p' roles/tackle/templates/customresource-schema.yml.j2
  echo "=== Lines 70-130 ===" && sed -n '70,130p' roles/tackle/templates/customresource-schema.yml.j2
else
  echo "File not found"
fi

Length of output: 2383


🏁 Script executed:

#!/bin/bash
# Search for additionalProperties or other structural hints
grep -n "additionalProperties\|oneOf\|anyOf\|allOf\|patternProperties" roles/tackle/templates/customresource-schema.yml.j2 2>/dev/null | head -20

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Look for references to the schema in code that might show how it's used
rg -n "Coordinates|organizations.*spaces|filter" --type go --type py -B3 -A3 | head -150

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Check for any existing nested structure definitions
grep -n "oneOf\|allOf\|items.*properties\|\$ref" roles/tackle/templates/customresource-schema.yml.j2 2>/dev/null

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Look at git show for current HEAD to see PR state
git show --stat | head -50

Length of output: 327


🏁 Script executed:

#!/bin/bash
# List diff for the specific file
git diff HEAD~1..HEAD -- roles/tackle/templates/customresource-schema.yml.j2 2>/dev/null || echo "No git history available"

Length of output: 2165


🏁 Script executed:

#!/bin/bash
# Check for Jinja2 filters or template logic that might affect structure
grep -n "{%\|{{\|filter\|loop" roles/tackle/templates/customresource-schema.yml.j2 | head -30

Length of output: 219


🏁 Script executed:

#!/bin/bash
# Look for supporting documentation
find . -name "*.md" -o -name "*.adoc" -o -name "*.txt" | xargs grep -l "Coordinates\|schema\|filter" 2>/dev/null | head -10

Length of output: 0


Critical: filter schema replaced nested organization→spaces→applications with flat arrays — restore nesting or confirm intentional breaking change

The change in roles/tackle/templates/customresource-schema.yml.j2 (lines ~87–109) removed the nested organization objects and replaced them with parallel arrays (organizations, spaces, names) and omitted required fields. This prevents expressing which spaces/apps belong to which organization and breaks the PR's stated "organizations with nested spaces and applications" behavior.

  • Restore the nested schema (organizations: array of objects { name: string, spaces: array of objects { name: string, applications: array[string] } }) so relationships are preserved.
  • Re-add appropriate required constraints (at minimum make the root organizations required and ensure application name is required where intended), or explicitly document and implement an alternative model that encodes relationships.

roles/tackle/templates/customresource-schema.yml.j2: lines 87–109.

🤖 Prompt for AI Agents
In roles/tackle/templates/customresource-schema.yml.j2 around lines 87–109 the
schema was flattened to parallel arrays (organizations, spaces, names) which
loses the relationship between org→space→application; revert to a nested schema:
make "organizations" a required property that is an array of objects, each
organization object must require "name" (string, minLength:1) and "spaces"
(array of objects), each space object must require "name" (string, minLength:1)
and "applications" (array of strings, items string minLength:1) so
org→spaces→applications relationships are preserved; ensure required
arrays/properties are re-added at each level and validate types/constraints
match existing conventions in the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is intentional. The schema provides for 3 flat optional lists.

Copy link

Choose a reason for hiding this comment

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

@jortel, thank you for the clarification! I understand now that the flat structure with three optional lists is the intended design for the filter schema.

I've noted this for future reference so we don't raise similar concerns about this design choice.


✏️ Learnings added
Learnt from: jortel
Repo: konveyor/operator PR: 484
File: roles/tackle/templates/customresource-schema.yml.j2:87-109
Timestamp: 2025-11-04T17:06:56.256Z
Learning: In roles/tackle/templates/customresource-schema.yml.j2, the cloudfoundry filter schema intentionally uses three flat, optional arrays (organizations, spaces, names) rather than a nested structure. This flat design is the correct implementation despite the PR description mentioning hierarchical organization→spaces→applications relationships.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Loading