RHDH-Local and Orchestrator Integration - #55
Conversation
Reviewer's GuideIntegrate Orchestrator plugins and the Sonataflow workflow engine into RHDH Local, providing out-of-the-box sample workflows and updated documentation for local development and testing. Entity relationship diagram for workflow example schemaserDiagram
GREETING_WORKFLOW {
string id
string version
string specVersion
string name
string description
string dataInputSchema
string start
object[] functions
object[] states
}
SLACK_WORKFLOW {
string id
string version
string specVersion
string name
string description
string dataInputSchema
string start
object[] functions
object[] states
}
GITHUB_WORKFLOW {
string id
string version
string specVersion
string name
string description
string dataInputSchema
string start
object[] functions
object[] states
}
GREETING_WORKFLOW ||--o| GREETING_INPUT_SCHEMA : uses
SLACK_WORKFLOW ||--o| SLACK_INPUT_SCHEMA : uses
GITHUB_WORKFLOW ||--o| GITHUB_INPUT_SCHEMA : uses
GITHUB_WORKFLOW ||--o| GITHUB_OPENAPI_SPEC : uses
SLACK_WORKFLOW ||--o| SLACK_OPENAPI_SPEC : uses
Class diagram for Orchestrator plugin configuration in dynamic-plugins.yamlclassDiagram
class DynamicPluginsConfig {
+includes: string[]
+plugins: PluginConfig[]
}
class PluginConfig {
+package: string
+integrity: string
+disabled: bool
+pluginConfig: object
}
class OrchestratorFrontendConfig {
+appIcons: object[]
+dynamicRoutes: object[]
}
class OrchestratorBackendConfig {
+dataIndexService: object
}
class ScaffolderOrchestratorConfig {
+dataIndexService: object
}
DynamicPluginsConfig "1" -- "*" PluginConfig
PluginConfig <|-- OrchestratorFrontendConfig
PluginConfig <|-- OrchestratorBackendConfig
PluginConfig <|-- ScaffolderOrchestratorConfig
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
4bde23e to
a9d6dff
Compare
d7ee7db to
7a4bf1b
Compare
There was a problem hiding this comment.
Hey @jenniferubah - I've reviewed your changes - here's some feedback:
- Consider externalizing the orchestrator plugin version and download URLs into variables or a single config entry to simplify future upgrades and avoid duplication across dynamic-plugins.yaml.
- Move sensitive values like NOTIFICATIONS_BEARER_TOKEN out of compose.yaml into an env file or secret management solution to prevent credentials from being checked into source control.
- Add a Docker healthcheck (or readiness probe) for the sonataflow service to ensure it’s fully ready before other services or clients attempt to connect.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider externalizing the orchestrator plugin version and download URLs into variables or a single config entry to simplify future upgrades and avoid duplication across dynamic-plugins.yaml.
- Move sensitive values like NOTIFICATIONS_BEARER_TOKEN out of compose.yaml into an env file or secret management solution to prevent credentials from being checked into source control.
- Add a Docker healthcheck (or readiness probe) for the sonataflow service to ensure it’s fully ready before other services or clients attempt to connect.
## Individual Comments
### Comment 1
<location> `README.md:68` </location>
<code_context>
- - Add your catalog entity overrides:
+ Note: In RHDH 1.7, by default, Orchestrator plugins are integrated in the dynamic plugins. If you intend to use these
+ plugins, ensure to copy the
+ orchestrator dynamic plugins from `dynamic-plugins.yamml` to your `dynamic-plugins.override.yaml` version for your local
+ development.
+
</code_context>
<issue_to_address>
Typo: 'yamml' should be 'yaml'.
The file extension should be corrected to 'yaml'.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
orchestrator dynamic plugins from `dynamic-plugins.yamml` to your `dynamic-plugins.override.yaml` version for your local
development.
=======
orchestrator dynamic plugins from `dynamic-plugins.yaml` to your `dynamic-plugins.override.yaml` version for your local
development.
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `rhdho-workflow-examples/README.md:12` </location>
<code_context>
+### Prerequisites
+
+* An existing workspace in slack.
+* An existing Slack App and Bot Token with the correct scopes.
+* Ensure the app is invited to the public channel.
+ Follow [slack guide](https://api.slack.com/tutorials/tracks/getting-a-token) on setting up slack app and bot token.
</code_context>
<issue_to_address>
Consider clarifying 'the correct scopes'.
Please list the specific scopes needed for the Slack Bot Token.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
* An existing Slack App and Bot Token with the correct scopes.
* Ensure the app is invited to the public channel.
Follow [slack guide](https://api.slack.com/tutorials/tracks/getting-a-token) on setting up slack app and bot token.
=======
* An existing Slack App and Bot Token with the following required scopes:
* `chat:write`
* `channels:read`
* `groups:read`
* `im:read`
* `mpim:read`
* Ensure the app is invited to the public channel.
Follow [slack guide](https://api.slack.com/tutorials/tracks/getting-a-token) on setting up slack app and bot token.
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `rhdho-workflow-examples/README.md:13` </location>
<code_context>
+
+* An existing workspace in slack.
+* An existing Slack App and Bot Token with the correct scopes.
+* Ensure the app is invited to the public channel.
+ Follow [slack guide](https://api.slack.com/tutorials/tracks/getting-a-token) on setting up slack app and bot token.
+
</code_context>
<issue_to_address>
Consider specifying how to invite the app.
Adding a short instruction or link on inviting the app to a channel would improve clarity.
</issue_to_address>
## Security Issues
### Issue 1
<location> `rhdho-workflow-examples/specs/github-openapi.json:1` </location>
<issue_to_address>
**security (CKV_OPENAPI_4):** Ensure that the global security field has rules defined
<details>
<summary>Description</summary>
OpenAPI uses security schemes to reference authentication and authorization schemes.
Your APIs should have authentication schemes in place and documented in the OpenAPI specification, as well as applied to individual operations or the entire API in the security details.
</details>
<details>
<summary>Fix - Buildtime</summary>
*OpenAPI*
Ensure that you have a securityScheme component and application.
For example:
```yaml
components:
security:
```
</details>
*Source: checkov*
</issue_to_address>
### Issue 2
<location> `rhdho-workflow-examples/specs/github-openapi.json:52` </location>
<issue_to_address>
**security (CKV_OPENAPI_3):** Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
<details>
<summary>Description</summary>
Sending credentials over HTTP in cleartext exposes API calls to attacks including man-in-the-middle attacks.
Ensure that you are using an encrypted channel for sending credentials.
</details>
<details>
<summary>Fix - Buildtime</summary>
*OpenAPI*
Ensure that you aren't using the unencryptedScheme.
For example:
```yaml
components:
securitySchemes:
- unencryptedScheme:
- type: http
- scheme: basic
paths:
"/":
get:
security:
- - unencryptedScheme: []
```
</details>
*Source: checkov*
</issue_to_address>
### Issue 3
<location> `rhdho-workflow-examples/specs/slack-openapi.json:1` </location>
<issue_to_address>
**security (CKV_OPENAPI_4):** Ensure that the global security field has rules defined
<details>
<summary>Description</summary>
OpenAPI uses security schemes to reference authentication and authorization schemes.
Your APIs should have authentication schemes in place and documented in the OpenAPI specification, as well as applied to individual operations or the entire API in the security details.
</details>
<details>
<summary>Fix - Buildtime</summary>
*OpenAPI*
Ensure that you have a securityScheme component and application.
For example:
```yaml
components:
security:
```
</details>
*Source: checkov*
</issue_to_address>
### Issue 4
<location> `rhdho-workflow-examples/specs/slack-openapi.json:51` </location>
<issue_to_address>
**security (CKV_OPENAPI_3):** Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
<details>
<summary>Description</summary>
Sending credentials over HTTP in cleartext exposes API calls to attacks including man-in-the-middle attacks.
Ensure that you are using an encrypted channel for sending credentials.
</details>
<details>
<summary>Fix - Buildtime</summary>
*OpenAPI*
Ensure that you aren't using the unencryptedScheme.
For example:
```yaml
components:
securitySchemes:
- unencryptedScheme:
- type: http
- scheme: basic
paths:
"/":
get:
security:
- - unencryptedScheme: []
```
</details>
*Source: checkov*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
217fcd8 to
177d9d5
Compare
|
@jenniferubah can yo resolve conflict pls so we can move closer to merging the PR? |
177d9d5 to
da0eb7c
Compare
|
| -f compose-with-orchestrator.yaml \ | ||
| up -d | ||
| ``` | ||
| Ensure to copy the orchestrator dynamic plugins from `dynamic-plugins-orchestrator.yaml` to your `dynamic-plugins.override.yaml` |
There was a problem hiding this comment.
Wondering if this could not be done automagically, as long as the compose-with-orchestrator.yaml is being used? With the help of the script that the init container runs, I feel like we could probably merge the content of dynamic-plugins-orchestrator.yaml and any additional user-provided plugins in dynamic-plugins.override.yaml (or the default one).
I see this new compose file as the Orchestrator flavor in RHDH Local, so IMO, users shouldn't have to explicitly add the Orchestrator plugins again.
WDYT? If too complicated, we can also leave this for a future improvement.
There was a problem hiding this comment.
I think it makes sense to automate the merge of the dynamic plugins files when the user wants to use the orchestrator compose yaml file instead of having them do it manually. And it might be complicated for this PR. I could do it in the next PR though.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
a51ffda to
0a76a26
Compare
0a76a26 to
2481369
Compare
|
/cherry-pick release-1.7 |
|
@rm3l: new pull request created: #69 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Description
The goal of this PR is to introduce the Orchestrator plugins and workflow infrastructure within the RHDH-Local environment, which is part of the RHDH Merge effort. This will enable users to have a local standalone environment where workflows can be easily developed and tested before they are deployed. This implementation adds the Orchestrator plugins (as part of default plugins) in the
dynamic-plugins.yamland uses the sonataflow devmode image to run the workflow engine.Which issue(s) does this PR fix or relate to
PR acceptance criteria
How to test changes / Special notes to the reviewer
To test these changes, run
podman-compose up -d. This should spin up RHDH and Sonataflow pods and accessible onlocalhost:7007. Please follow theTesting Orchestrator Workflow Examplessection in the README.md to run workflows.Known Issues
Summary by Sourcery
Enable local development and testing of Orchestrator workflows in RHDH Local by integrating Orchestrator plugins, adding a Sonataflow workflow engine service, providing sample workflows, and updating documentation and default configurations.
New Features:
Enhancements:
Documentation:
Summary by Sourcery
Enable local development and testing of Orchestrator workflows by integrating Orchestrator plugins into RHDH Local, adding a Sonataflow devmode service, supplying sample workflows, and updating configuration and documentation.
New Features:
Enhancements:
Documentation: