Skip to content

Add/workflowstep definition fixes#11

Merged
anoop2811 merged 44 commits into
kubevela:mainfrom
roguepikachu:add/workflowstep-definition-fixes
Mar 18, 2026
Merged

Add/workflowstep definition fixes#11
anoop2811 merged 44 commits into
kubevela:mainfrom
roguepikachu:add/workflowstep-definition-fixes

Conversation

@roguepikachu
Copy link
Copy Markdown
Contributor

@roguepikachu roguepikachu commented Mar 12, 2026

Description of your changes

copilot:all

Workflow Step Definition Fixes:

1. vela-cli

  • Replaced raw builtin.#Fail CUE reference with defkit.Fail() fluent builder
  • Replaced manual builtin.#ConditionalWait builder calls with defkit.WaitUntil().Guard() fluent builder for job status wait
  • mountsArray and volumesList use fluent ForEachGuarded builders (requires framework fix for inner braces)
  • deDupVolumesArray uses defkit.From(defkit.Reference("volumesList")).Dedupe("name") (requires framework fix for Ref source support)
  • Removed unused hasJobSucceededStatus variable

2. step-group

  • Added TemplateBody("// no parameters, the nop only to make the template not empty or it's invalid\nnop: {}") to replace empty parameter: {} with a valid nop placeholder
  • Leverages framework change that skips parameter block when TemplateBody is set with no params

3. share-cloud-resource

  • Used WithDirectFields() to render op.#ShareCloudResource fields (env, placements, policy) directly without $params wrapper
  • Restored defkit.VelaCtx() fluent builder usage

4. webhook

  • Rewrote using fluent builders: KubeRead, HTTPPost, ConvertString for cleaner templates
  • Made url parameter a closed union (value or secretRef)
  • Correctly reads Application/Secret and POSTs JSON

5. suspend

  • Added trailing newline for proper formatting

6. Test coverage

  • New vela_cli_test.go: 59 specs covering metadata, imports, parameters (command, image, serviceAccountName, storage), template blocks (mountsArray, volumesList, deDupVolumesArray, job, log, fail, wait)
  • New step_group_test.go: 7 specs covering nop placeholder and parameter suppression
  • New share_cloud_resource_test.go: 17 specs covering direct field rendering
  • New webhook_test.go: comprehensive specs covering metadata, CUE generation, and closed union parameters

Dependency:

Requires kubevela/kubevela#7069 for framework changes (ForEachGuarded inner braces, Dedupe for Ref sources, WithDirectFields, TemplateBody parameter suppression).

How has this code been tested

  • All workflow step unit tests pass (go test -v -race -count=1 ./workflowsteps/...)
  • Tests use Ginkgo/Gomega and validate generated CUE output against expected patterns

Special notes for your reviewer

All definition changes maximize fluent builder usage over raw CUE strings. Each fix was driven by comparing defkit-generated CUE against the original hand-written CUE definitions in translations/.

@roguepikachu roguepikachu force-pushed the add/workflowstep-definition-fixes branch 2 times, most recently from 383b4d4 to 974ba61 Compare March 17, 2026 04:48
@roguepikachu
Copy link
Copy Markdown
Contributor Author

DO NOT MERGE - Dependency on KubeVela PR kubevela/kubevela#7069

…t disjunction

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…ethods for improved clarity

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
… functionality

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
… in workflow step definition

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…older and add tests for metadata and CUE generation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
… fields and add comprehensive tests for metadata and CUE generation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…er formatting

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…owStep with metadata and CUE generation validation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…pdate registration

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…ata and CUE generation validation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…for metadata and CUE generation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…E generation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…nParam and enhance notifyChannelAction for improved notification handling

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
… and CUE generation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…nd CUE generation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…proved parameter handling

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…ced secret handling and conditional logic

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…accurate CUE output validation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…d tests for workflow step metadata and CUE generation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…ection workflow steps

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…d add tests for Deploy and ExportData workflow steps

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…ly for improved resource handling

test(workflowsteps): add comprehensive tests for DependsOnApp workflow step functionality and CUE generation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…g metadata and CUE generation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…l parameter object and add tests for CreateConfig WorkflowStep

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…rameters and improve filtering logic; add comprehensive tests for workflow step

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…etadata and CUE generation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…ail and wait logic; add comprehensive tests for metadata and CUE generation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…d wait logic; add comprehensive tests for metadata and CUE generation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…eter object; add tests for metadata and CUE generation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
… WorkflowSteps including metadata and CUE generation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…aformProvider WorkflowSteps including metadata and CUE generation

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
- Updated ApplyTerraformProvider tests to ensure all 8 providers have default names and type constraints.
- Improved BuildPushImage tests to verify optional credentials struct includes git and image substructs.
- Added checks in CleanJobs tests for label selector and default matching labels.
- Enhanced DeployCloudResource tests to include app action name in the cue output.
- Added validation in ListConfig tests to ensure exactly one config.#ListConfig is present.
- Updated Notification tests to verify ClosedUnion structure for password field.
- Enhanced PrintMessageInStatus tests to ensure correct description and single instance of builtin.#Message.
- Improved ReadConfig tests to ensure exactly one config.#ReadConfig is present.
- Updated ReadObject tests to ensure correct description and single instance of kube.#Read.
- Enhanced Request tests to verify parameter types and structural correctness.
- Updated RestartWorkflow tests to ensure correct description and structural correctness.
- Enhanced ShareCloudResource tests to ensure required placements array is structured correctly.
- Added checks in StepGroup tests to ensure no parameter block or import statements are present.
- Updated Suspend tests to ensure correct description and structural correctness.
- Enhanced workflowsteps_test to ensure all workflow steps are registered with correct names and descriptions.

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…flow steps

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
… in provider definitions; update tests for ClosedUnion structures in webhook and notification steps

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…pdate tests for optional fields in Sidecar and K8sUpdateStrategy traits

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…o use correct return value syntax

fix(traits): add workload reference path and conflict handling in K8sUpdateStrategy definition

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
…to use proper return value

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
… 1.10.5-0.20260316133806-c6ee19ee45d9

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
… version 1.10.5-0.20260316133806-c6ee19ee45d9"

This reverts commit 35dd374.

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
@roguepikachu roguepikachu force-pushed the add/workflowstep-definition-fixes branch from 0e39d1c to c033ced Compare March 17, 2026 14:14
Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

3 issues found across 59 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="workflowsteps/workflowsteps_test.go">

<violation number="1" location="workflowsteps/workflowsteps_test.go:45">
P2: The "All WorkflowSteps Registered" test relies on a manual step list and is already missing registered workflow steps, so it does not truly verify registration completeness.</violation>
</file>

<file name="workflowsteps/clean_jobs_test.go">

<violation number="1" location="workflowsteps/clean_jobs_test.go:116">
P2: Test asserts lowercase `kind: "pod"`, which conflicts with Kubernetes Kind conventions (CamelCase) and may lock in an invalid Kind for pod deletion.</violation>
</file>

<file name="workflowsteps/collect_service_endpoints_test.go">

<violation number="1" location="workflowsteps/collect_service_endpoints_test.go:222">
P2: App-block parsing in test is brace-naive and exits at inner `}`, allowing forbidden guards later in `app` to go unchecked.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread workflowsteps/workflowsteps_test.go
Comment thread workflowsteps/clean_jobs_test.go Outdated
Comment thread workflowsteps/collect_service_endpoints_test.go Outdated
- Consolidated metadata tests for ShareCloudResource, StepGroup, Suspend, VelaCli, and Webhook steps to check name and description in a single test.
- Simplified CUE generation tests by combining related assertions into fewer tests for clarity.
- Ensured consistent formatting and improved test descriptions across all workflow step tests.
- Verified structural correctness and parameter declarations in CUE outputs for each workflow step.

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

7 issues found across 31 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="workflowsteps/build_push_image_test.go">

<violation number="1" location="workflowsteps/build_push_image_test.go:60">
P2: Credential schema assertions were removed, reducing test coverage for required secret fields (`name`/`key`) in `#secret` and nested credential blocks.</violation>

<violation number="2" location="workflowsteps/build_push_image_test.go:115">
P2: Consolidated lifecycle test no longer verifies that `read: kube.#Read` targets `kind: "Pod"`, allowing read-kind regressions to pass.</violation>
</file>

<file name="workflowsteps/apply_terraform_provider_test.go">

<violation number="1" location="workflowsteps/apply_terraform_provider_test.go:112">
P2: Refactor removed explicit assertions for several required provider env-key mappings, weakening regression coverage for ApplyTerraformProvider config generation.</violation>
</file>

<file name="workflowsteps/restart_workflow_test.go">

<violation number="1" location="workflowsteps/restart_workflow_test.go:64">
P2: Test assertions were weakened: they no longer explicitly verify `if parameter.<x> != _|_` branch guards for `_script`, so branch-generation regressions may go undetected.</violation>
</file>

<file name="workflowsteps/request_test.go">

<violation number="1" location="workflowsteps/request_test.go:63">
P2: Test consolidation removed assertion of the `op.#Fail` message content, weakening regression coverage for user-visible request failure diagnostics.</violation>
</file>

<file name="workflowsteps/vela_cli_test.go">

<violation number="1" location="workflowsteps/vela_cli_test.go:127">
P2: Refactored test weakens log selector validation by checking only `labelSelector:` presence and no longer asserting the required `workflow.oam.dev/step-name` key.</violation>

<violation number="2" location="workflowsteps/vela_cli_test.go:134">
P2: Refactored test removed wait-specific guard assertions, weakening coverage for safe `status.succeeded` dereference.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread workflowsteps/build_push_image_test.go
Comment thread workflowsteps/build_push_image_test.go
Comment thread workflowsteps/apply_terraform_provider_test.go
Comment thread workflowsteps/restart_workflow_test.go Outdated
Comment thread workflowsteps/request_test.go
Comment thread workflowsteps/vela_cli_test.go
Comment thread workflowsteps/vela_cli_test.go
…est assertions

Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com>
@anoop2811 anoop2811 merged commit 1486fb4 into kubevela:main Mar 18, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants