|
| 1 | +# 0004 Implementation Plan |
| 2 | + |
| 3 | +## Goal |
| 4 | + |
| 5 | +- Deliver a real Slack-only `send-message` StepPlugin under |
| 6 | + `extended/plugins/send-message`. |
| 7 | +- Keep that subtree self-contained enough to behave like its own Git repo. |
| 8 | +- Do not rely on code, files, build helpers, tests, or other resources outside |
| 9 | + `extended/plugins/send-message` for the plugin implementation itself. |
| 10 | +- Use the StepPlugin host slice from proposal |
| 11 | + [0002-kargo-executor-plugins-from-argo-workflows](../0002-kargo-executor-plugins-from-argo-workflows/proposal.md) |
| 12 | + without widening host responsibilities. |
| 13 | +- Keep channel lookup and referenced Secret reads in the plugin, not the host. |
| 14 | +- Use local-only Slack credentials for smoke testing. Do not commit any real |
| 15 | + token or local token source. |
| 16 | +- Make the plugin match the documented Slack subset, including encoded-message |
| 17 | + behavior, except for SMTP which stays out of scope. |
| 18 | + |
| 19 | +## Implementation Shape |
| 20 | + |
| 21 | +- Put all plugin-owned source, manifests, tests, and smoke assets under: |
| 22 | + - `extended/plugins/send-message/` |
| 23 | +- Treat that subtree as a standalone plugin repo: |
| 24 | + - own runtime source |
| 25 | + - own image build |
| 26 | + - own CRDs |
| 27 | + - own RBAC manifests |
| 28 | + - own tests |
| 29 | + - own smoke-test helpers |
| 30 | + - own smoke entrypoint script |
| 31 | +- It is acceptable for repo-level smoke harness code outside that subtree to |
| 32 | + call into the subtree's smoke entrypoint. |
| 33 | +- It is not acceptable for plugin runtime code inside that subtree to import or |
| 34 | + shell out to helpers elsewhere in this repo. |
| 35 | + |
| 36 | +## Plugin Runtime |
| 37 | + |
| 38 | +- Build a plugin-owned runtime image from `extended/plugins/send-message/`. |
| 39 | +- Implement `POST /api/v1/step.execute`. |
| 40 | +- Enforce the StepPlugin bearer-token contract: |
| 41 | + - read `/var/run/kargo/token` |
| 42 | + - require `Authorization: Bearer ...` |
| 43 | + - return `403` on bad auth |
| 44 | +- Read Kubernetes credentials from the mounted service account projection when |
| 45 | + present. |
| 46 | +- Use direct Kubernetes API reads from the plugin: |
| 47 | + - `MessageChannel` in the Project namespace |
| 48 | + - `ClusterMessageChannel` cluster-scoped |
| 49 | + - referenced `Secret` in the Project namespace or system-resources namespace |
| 50 | +- Call Slack from the plugin runtime, not through the host. |
| 51 | + |
| 52 | +## V1 Behavior |
| 53 | + |
| 54 | +- Scope: |
| 55 | + - `send-message` |
| 56 | + - `MessageChannel` |
| 57 | + - `ClusterMessageChannel` |
| 58 | + - Slack only |
| 59 | +- Do not implement SMTP. |
| 60 | +- Support this step-config subset in v1: |
| 61 | + - `channel.kind` |
| 62 | + - `channel.name` |
| 63 | + - `message` |
| 64 | + - `encodingType` |
| 65 | + - `slack.channelID` |
| 66 | + - `slack.threadTS` |
| 67 | +- Support this output subset in v1: |
| 68 | + - `slack.threadTS` |
| 69 | +- Plaintext behavior: |
| 70 | + - `message` is sent as Slack `text` |
| 71 | + - `slack.channelID` overrides channel resource `spec.slack.channelID` |
| 72 | + - `slack.threadTS` is sent as Slack `thread_ts` |
| 73 | + - output `slack.threadTS` is `config.slack.threadTS` if set, else Slack |
| 74 | + response `ts` |
| 75 | +- Encoded-message behavior: |
| 76 | + - support `json`, `yaml`, and `xml` |
| 77 | + - when `encodingType` is set, treat the body as the Slack payload object |
| 78 | + - ignore `config.slack.channelID` and `config.slack.threadTS` |
| 79 | + - if encoded body omits `channel`, fill it from channel resource |
| 80 | + `spec.slack.channelID` |
| 81 | + - if encoded body sets `thread_ts`, return that value in output |
| 82 | + - otherwise return Slack response `ts` in output |
| 83 | +- XML behavior: |
| 84 | + - XML is a Kargo-owned alternate serialization of the same Slack payload |
| 85 | + object used for `json` and `yaml` |
| 86 | + - root element name is ignored |
| 87 | + - attributes become keys |
| 88 | + - repeated sibling element names become arrays |
| 89 | + - nested elements become nested objects |
| 90 | +- `MessageChannel` lookup rules: |
| 91 | + - resource kind is `MessageChannel` |
| 92 | + - resource namespace is the Project namespace from the step context |
| 93 | + - `secretRef.name` resolves in the same namespace |
| 94 | + - `spec.slack.channelID` is required |
| 95 | + - Slack token key is `apiKey` |
| 96 | +- `ClusterMessageChannel` lookup rules: |
| 97 | + - resource kind is `ClusterMessageChannel` |
| 98 | + - `secretRef.name` resolves in the system-resources namespace configured for |
| 99 | + the plugin install |
| 100 | + - `spec.slack.channelID` is required |
| 101 | + - Slack token key is `apiKey` |
| 102 | + |
| 103 | +## CRDs And RBAC |
| 104 | + |
| 105 | +- The plugin subtree owns the OSS CRD manifests for: |
| 106 | + - `MessageChannel` |
| 107 | + - `ClusterMessageChannel` |
| 108 | +- Use the existing Akuity API group: |
| 109 | + - `ee.kargo.akuity.io/v1alpha1` |
| 110 | +- Keep schemas minimal but structurally valid for the Slack-only slice. |
| 111 | +- Ship plugin-owned RBAC manifests under the subtree. |
| 112 | +- For the local smoke path: |
| 113 | + - bind only the test Project default `ServiceAccount` |
| 114 | + - grant channel reads and referenced Secret reads needed by the plugin |
| 115 | + - keep that setup in the smoke assets or smoke helper, not in host code |
| 116 | + |
| 117 | +## Tests |
| 118 | + |
| 119 | +- Keep plugin tests under `extended/plugins/send-message/`. |
| 120 | +- Cover at least: |
| 121 | + - bearer-token auth |
| 122 | + - `MessageChannel` lookup |
| 123 | + - `ClusterMessageChannel` lookup |
| 124 | + - referenced Secret lookup |
| 125 | + - Slack request shaping for plain text |
| 126 | + - `slack.channelID` override |
| 127 | + - `slack.threadTS` override |
| 128 | + - `slack.threadTS` output |
| 129 | + - encoded-body behavior ignores `config.slack.*` |
| 130 | + - `xml` decode path |
| 131 | + - failure path when channel or Secret is missing |
| 132 | + - failure path when Slack returns an error |
| 133 | + |
| 134 | +## Smoke Test |
| 135 | + |
| 136 | +- Put the primary smoke script in: |
| 137 | + - `extended/plugins/send-message/smoke/smoke-test.sh` |
| 138 | +- That script assumes Kargo is already available and takes its inputs from env. |
| 139 | +- Keep the committed smoke path credential-free. |
| 140 | +- Expect a local env var for the Slack API token during local smoke testing. |
| 141 | +- Build the plugin image from `extended/plugins/send-message/`. |
| 142 | +- Load that image into the local kind cluster without touching the user's |
| 143 | + global kube context. |
| 144 | +- Install plugin CRDs, RBAC, and generated StepPlugin `ConfigMap`. |
| 145 | +- Create either a `MessageChannel` or `ClusterMessageChannel` test resource and |
| 146 | + the referenced Secret in-cluster from the local-only token source. |
| 147 | +- Run a `Stage` with `uses: send-message`. |
| 148 | +- Prove in-cluster success from `Promotion` status, including non-empty |
| 149 | + `slack.threadTS`. |
| 150 | +- For interactive local verification, also confirm the message appeared in the |
| 151 | + target Slack channel. |
| 152 | +- The repo harness may call this script at the right point in e2e flow, but the |
| 153 | + smoke orchestration lives in the plugin subtree. |
| 154 | + |
| 155 | +## Host Changes |
| 156 | + |
| 157 | +- Prefer zero host changes. |
| 158 | +- If the plugin proves a host gap, keep any host fix thin and behind |
| 159 | + `extended/`. |
| 160 | +- Do not move channel lookup, Secret reads, or Slack API calls into the host. |
| 161 | +- Host gap actually found: |
| 162 | + - non-root sidecars could not read `/var/run/kargo/token` |
| 163 | + - keep the fix thin in `extended/pkg/stepplugin/agent/command_bridge.go` |
| 164 | + - auth directory mode must permit traversal by non-root sidecars |
| 165 | + - auth file mode must permit read by non-root sidecars |
| 166 | + |
| 167 | +## Phase Post-Green: Minimize Diff Of Files Outside ./extended Against Kargo Upstream |
| 168 | + |
| 169 | +1. Get the feature green first. |
| 170 | +2. Fetch `upstream/main`. |
| 171 | +3. Review every edited file outside `extended/`, if any. |
| 172 | +4. Move logic behind `extended/` helpers where that shrinks the conflict |
| 173 | + surface. |
| 174 | +5. Re-run the matching tests after each cleanup pass. |
| 175 | +6. Stop when no obvious helper extraction or edit-block reduction remains. |
0 commit comments