Feat/plugin down kubernetes required by maya-ots · Pull Request #67 · MicroClub-USTHB/mctf-cli

maya-ots · 2026-10-23T00:02:16Z

What

Extend plugin hook dispatch to teardown and Kubernetes deployment paths, and add opt-in required plugins that can block operations when their hooks fail.

Closes #42

How

Added before_down and after_down hooks for mctf down, keeping teardown semantics distinct from deployment hooks.
Wired plugin hooks into the Kubernetes applyKube path, dispatching deploy hooks for up and teardown hooks for down.
Added the required plugin configuration option, defaulting to false.
Updated plugin loading and dispatch behavior so failures from required plugins can veto operations, while non-required plugins remain best-effort.
Preserved the existing "before" and "after" phase values and added "kubernetes" as a supported deployment mode in the payload documentation.
Updated the configuration schema copies and docs/spec/plugins.md to document required-plugin behavior and the additional hooks.
Added tests covering the required-plugin veto path and teardown hook delivery.
Ensured successful teardown hooks are not dispatched for dry-run operations.

Verification

go build ./...
go test ./...
golangci-lint run ./...
mctf generate --check on the sample event (if derived files could change)
Docs updated (docs/spec/plugins.md)
Schema updated in both schema copies
make schemas synced and verified
go test ./internal/plugin ./cmd — passed

Notes

Required plugins intentionally change failure behavior only when required: true is configured; the default remains best-effort.
Required-plugin failures can block operations at the relevant pre-operation hook.
The focused plugin and command tests pass. The full test suite, build, lint, and schema-generation checks have not yet been confirmed.

ademboukabes

Overall solid work on extending plugin hooks to teardown and Kubernetes paths and adding required plugin support.

All automated checks pass locally (build, unit tests, race detector, lint, schema sync, sample-event validation).

Leaving a few inline comments on payload consistency, error-handling conventions, doc cleanup, and test coverage.

ademboukabes · 2026-10-30T06:27:14Z

+		p.Error = err.Error()
+	}
+	for _, u := range plan {
+		p.Units = append(p.Units, plugin.DeployUnit{


In kubeDeployHookPayload, only Challenge is set. The fields Category, Slug, ProjectName, and ImageTag are left empty, unlike in deployHookPayload.

Please map the remaining fields from u so Kubernetes hook payloads remain consistent with Docker and SSH modes:

p.Units = append(p.Units, plugin.DeployUnit{ Challenge: u.Challenge, Category: u.Category, Slug: u.Slug, ProjectName: u.Name, ImageTag: u.Image, })

ademboukabes · 2026-10-30T06:27:14Z

 		}

-		pm.Dispatch(ctx, plugin.HookAfterBuild, buildHookPayload(cfg, opts, "after", flagBuildDryRun, plan, "success", nil))
+		if err := pm.Dispatch(ctx, plugin.HookAfterBuild, buildHookPayload(cfg, opts, "after", flagBuildDryRun, plan, "success", nil)); err != nil {


There is an inconsistency in how after_* hook failures are handled across commands:

In cmd/build.go, an error returned by a required plugin during after_build causes the command to exit with an error.

In cmd/deploy.go (lines 233, 423, 572), errors from after_deploy and after_down are discarded with _ =.

We should align the behavior across all commands:

If after_* hooks are non-blocking as stated in docs/spec/plugins.md ("Failures in after_* hooks do not undo an operation that has already completed"), change this to _ = pm.Dispatch(...).

If failing after_* hooks should report an error to CI, check and return the error in cmd/deploy.go as well.

ademboukabes · 2026-10-30T06:27:14Z

+// its own DispatchTimeout. Optional plugin failures are logged and dropped.
+// Failures from required plugins are also returned so blocking hooks can veto
+// the host operation.
+func (m *Manager) Dispatch(ctx context.Context, hook string, payload any) error {


The doc comment directly above Dispatch is duplicated. The first paragraph ("It never returns an error.") is outdated and contradicts the updated comment and the new error return type. Please remove the first paragraph.

ademboukabes · 2026-10-30T06:27:14Z

 		}
 		if err := p.start(procCtx, opts.Dir); err != nil {
 			fmt.Fprintf(log, "warning: plugin %q failed to start: %v\n", c.Name, err)
+			if c.Required && m.loadErr == nil {


Two notes regarding initialization and error handling:

Load doc comment (lines 39-43) is now partially outdated since it claims Load never aborts the host command and returns nil when no plugin is live. With m.loadErr, Load now retains startup/handshake errors for required plugins. Please update the doc comment.

If ParseConfigs fails on line 53, Load returns nil and the host command proceeds unblocked. If any raw plugin entry had required: true, this creates a fail-open condition. If a required plugin cannot be parsed, Load should retain the error in m.loadErr so Dispatch can block the operation.

ademboukabes · 2026-10-30T06:27:14Z

 	m.Close()
 }
+
+func TestManager_RequiredPluginCanVetoDeploy(t *testing.T) {


Please consider adding two test cases here:

A test covering startup or handshake failure for a plugin with required: true, verifying that m.loadErr blocks subsequent Dispatch calls.

A test for kubeDeployHookPayload verifying that all fields in DeployUnit (Category, Slug, ProjectName, ImageTag) are correctly populated.

ademboukabes

Requesting changes based on the inline review comments:

cmd/deploy.go: Populate the missing DeployUnit fields (Category, Slug, ProjectName, ImageTag) in kubeDeployHookPayload so Kubernetes payloads match Docker/SSH.
cmd/build.go vs cmd/deploy.go: Harmonize error-handling conventions on after_* hooks across commands.
internal/plugin/manager.go: Remove duplicated doc comment on Dispatch, update Load docstring, and avoid fail-open behavior if ParseConfigs fails when a required plugin is declared.
internal/plugin/plugin_test.go: Add unit tests for required plugin startup/handshake failure and kubeDeployHookPayload mapping.

maya-ots · 2026-10-30T20:08:26Z

thank you for the review!! i'll make the changes right away

maya-ots · 2026-10-31T10:56:13Z

all request changes done + added the corresponding tests, all tests pass

maya-ots added 2 commits October 22, 2026 08:46

feat(plugin): support required plugin failures

5104ed9

feat(plugin): dispatch teardown and Kubernetes hooks

8277602

maya-ots linked an issue Oct 23, 2026 that may be closed by this pull request

plugin: dispatch on down/kubernetes paths + blocking (required) mode #42

Open

fix(plugin): handle dispatch errors

9076af5

ademboukabes reviewed Oct 30, 2026

View reviewed changes

ademboukabes requested changes Oct 30, 2026

View reviewed changes

maya-ots added 2 commits October 31, 2026 02:10

fix(plugin): propagate required hook failures

1203a67

test(plugin): cover required plugin failures

c429935

ademboukabes merged commit 57caa9b into develop Oct 31, 2026
3 checks passed

ademboukabes deleted the feat/plugin-down-kubernetes-required branch October 31, 2026 13:33

Navigation Menu

Repository navigation

Feat/plugin down kubernetes required - #67

Feat/plugin down kubernetes required#67
ademboukabes merged 5 commits into
developfrom
feat/plugin-down-kubernetes-required

maya-ots commented Oct 23, 2026 •

edited

Loading

Uh oh!

ademboukabes left a comment

Uh oh!

ademboukabes Oct 30, 2026

Uh oh!

ademboukabes Oct 30, 2026

Uh oh!

ademboukabes Oct 30, 2026

Uh oh!

ademboukabes Oct 30, 2026

Uh oh!

ademboukabes Oct 30, 2026

Uh oh!

ademboukabes left a comment

Uh oh!

maya-ots commented Oct 30, 2026

Uh oh!

maya-ots commented Oct 31, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

maya-ots commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

How

Verification

Notes

Uh oh!

ademboukabes left a comment

Choose a reason for hiding this comment

Uh oh!

ademboukabes Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

ademboukabes Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

ademboukabes Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

ademboukabes Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

ademboukabes Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

ademboukabes left a comment

Choose a reason for hiding this comment

Uh oh!

maya-ots commented Oct 30, 2026

Uh oh!

maya-ots commented Oct 31, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

maya-ots commented Oct 23, 2026 •

edited

Loading