-
Notifications
You must be signed in to change notification settings - Fork 27
feat: add workato #492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add workato #492
Conversation
WalkthroughA new optional field Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
api/v1alpha1/zz_generated.openapi.go (1)
2662-2668: Generated schema OK, but please fix naming and add URL validation in the source before regen.
- Rename field/key to “WorkatoWebhookURL”/“workatoWebhookURL” for consistency with existing “Webhook*” identifiers.
- Add kubebuilder validation to mark it as a URI and enforce https.
- Re-run codegen so OpenAPI shows
format: uri(and pattern).After making the change in
toolchainconfig_types.go, re-runmake generateand check that this block showsworkatoWebhookURLandFormat: "uri".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
api/v1alpha1/docs/apiref.adoc(1 hunks)api/v1alpha1/toolchainconfig_types.go(1 hunks)api/v1alpha1/zz_generated.deepcopy.go(1 hunks)api/v1alpha1/zz_generated.openapi.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Verify Dependencies
🔇 Additional comments (2)
api/v1alpha1/zz_generated.deepcopy.go (1)
1967-1971: LGTM! Deep copy implementation is correct.The auto-generated deep copy logic for the new
WorkatoWebHookURLfield follows the standard Kubernetes pattern for optional pointer fields and is consistent with other similar fields in theRegistrationServiceConfigstruct.api/v1alpha1/docs/apiref.adoc (1)
2043-2044: Documentation for new Workato webhook field is clear and well-placed.The field documentation accurately reflects the purpose (analytics events to Marketo) and environment-specific nature. Formatting and positioning within the
RegistrationServiceConfigtable are consistent with existing field documentation patterns.
| // WorkatoWebHookURL is used by the UI to push events to Marketo for analytics purposes. | ||
| // The webhook URL is unique per environment. | ||
| // +optional | ||
| WorkatoWebHookURL *string `json:"workatoWebHookURL,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Rename to “Webhook” and add URL validation (Format=uri + https-only).
Avoid locking in an inconsistent public API and add basic validation hints.
Apply this diff:
@@
- // WorkatoWebHookURL is used by the UI to push events to Marketo for analytics purposes.
- // The webhook URL is unique per environment.
- // +optional
- WorkatoWebHookURL *string `json:"workatoWebHookURL,omitempty"`
+ // WorkatoWebhookURL is used by the UI to push events to Marketo for analytics purposes.
+ // The webhook URL is unique per environment.
+ // +kubebuilder:validation:Format=uri
+ // +kubebuilder:validation:Pattern=`^https://`
+ // +optional
+ WorkatoWebhookURL *string `json:"workatoWebhookURL,omitempty"`🏁 Script executed:
#!/bin/bash
# Verify no lingering references to the old name remain and that the new name exists.
set -euo pipefail
echo "Searching for old/new identifiers..."
rg -n -S '\bWorkatoWebHookURL\b|\bworkatoWebHookURL\b|\bWorkatoWebhookURL\b|\bworkatoWebhookURL\b' -C2
echo
echo "Note: run 'make generate' locally, then ensure:"
echo "- zz_generated.openapi.go shows property 'workatoWebhookURL' with Format: 'uri'"
echo "- zz_generated.deepcopy.go reflects WorkatoWebhookURL copies"Length of output: 2232
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Check for other webhook/hook field naming patterns in the types file
echo "=== Other webhook/hook related fields ==="
rg -n -i 'webhook|webhookconfig|webhooki' api/v1alpha1/toolchainconfig_types.go -C1
echo
echo "=== Kubebuilder validation examples in same file ==="
rg -n 'kubebuilder:validation:' api/v1alpha1/toolchainconfig_types.go | head -20
echo
echo "=== Check if Format=uri validation exists elsewhere ==="
rg -n 'Format=uri' api/v1alpha1/ -C1Length of output: 620
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Search for WebhookConfig and WebhookSecret across entire codebase
echo "=== Searching for WebhookConfig ==="
rg -n 'WebhookConfig' . -C1
echo
echo "=== Searching for WebhookSecret ==="
rg -n 'WebhookSecret' . -C1
echo
echo "=== Search for other URL fields with validation ==="
rg -n -A2 'Format.*uri|Pattern.*https' api/v1alpha1/ -C1Length of output: 8860
Rename to "Webhook" and add URL validation before API stabilizes.
The codebase establishes "Webhook" naming in WebhookConfig and WebhookSecret (memberoperatorconfig_types.go). The new field breaks this convention. Since this field is newly added, apply both the naming correction and validation markers now to avoid future API churn.
Apply this diff:
@@
- // WorkatoWebHookURL is used by the UI to push events to Marketo for analytics purposes.
- // The webhook URL is unique per environment.
- // +optional
- WorkatoWebHookURL *string `json:"workatoWebHookURL,omitempty"`
+ // WorkatoWebhookURL is used by the UI to push events to Marketo for analytics purposes.
+ // The webhook URL is unique per environment.
+ // +kubebuilder:validation:Format=uri
+ // +kubebuilder:validation:Pattern=`^https://`
+ // +optional
+ WorkatoWebhookURL *string `json:"workatoWebhookURL,omitempty"`After renaming, run make generate locally to ensure zz_generated.deepcopy.go and zz_generated.openapi.go reflect the updated field name and validation markers.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In api/v1alpha1/toolchainconfig_types.go around lines 262 to 265, rename the
struct field from WorkatoWebHookURL to WorkatoWebhookURL (use "Webhook" not
"WebHook"), update the json tag to `json:"workatoWebhookURL,omitempty"`, and add
validation markers such as `// +optional` and `//
+kubebuilder:validation:Format=uri` above the field; after making these changes
run `make generate` to regenerate zz_generated.deepcopy.go and
zz_generated.openapi.go so the new field name and validation are reflected.



Description
We would like to start pushing events from the UI dashbord for all our current Trial buttons to Marketo.
In order to do this we need to send the events to a configurable webhook URL which is unique per environment ( dev/stage/prod ) and doesn't have authentication. For those reasons we should make it configurable and fetch it at runtime.
This config value will be exposed using from the
/uiconfigendpoint : https://github.com/codeready-toolchain/registration-service/blob/b91359ddf1e6f702f28f675162af2f3c3928c258/pkg/server/routes.go#L101 in follow up pr's.Jira: https://issues.redhat.com/browse/SANDBOX-1484
Checks
Did you run
make generatetarget? yesDid
make generatechange anything in other projects (host-operator)? yesIn case of new CRD, did you the following? N/A
resources/setup/roles/host.yamlin the sandbox-sre repositoryPROJECTfile: https://github.com/codeready-toolchain/host-operator/blob/master/PROJECTCSVfile: https://github.com/codeready-toolchain/host-operator/blob/master/config/manifests/bases/host-operator.clusterserviceversion.yamlIn case other projects are changed, please provides PR links.
Summary by CodeRabbit