From fce92206f6a450eb63b2b3b4edc0ae5c02606a0a Mon Sep 17 00:00:00 2001 From: francisco-orkes Date: Wed, 5 Aug 2026 13:36:31 -0700 Subject: [PATCH 1/2] fix(scheduler): try PUT before GET for schedule pause/resume MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `conductor schedule pause` and `conductor schedule resume` fail against OSS Conductor with 405 "Request method 'GET' is not supported". The conductor-go SDK issues GET for both (v1.8.0 api_scheduler_resource.go:188 and :255), but upstream OSS declares them @PutMapping. The accepted verb varies by deployment, so neither verb alone is correct: OSS Conductor PUT only (GET -> 405) Orkes Conductor >= 2026-07-14 PUT or GET Orkes Conductor < 2026-07-14 GET only (PUT -> 405) Orkes gained PUT in orkes-conductor 1854375f0c ("accept PUT (not just GET) for per-schedule pause/resume", 2026-07-14), so deployments older than that still need GET. This tries PUT first and falls back to GET only on a 4xx, which satisfies all three and converges on the RESTful verb as older builds age out — at which point the fallback can be deleted. The fallback is deliberately limited to 4xx. Retrying a 5xx or a transport error with a different verb would mask the real fault and report a misleading "method not supported" instead. Implemented in the CLI rather than the SDK on purpose: hardcoding PUT in conductor-go would fix OSS and break every Orkes deployment predating the change. It bypasses SchedulerClient.PauseSchedule/ResumeSchedule via the shared APIClient, which already exposes both verbs, so no SDK bump is needed to unblock the release. Adds internal.GetAPIClient for that purpose, documented as a last resort for when a typed SDK client issues the wrong request. Tests cover all three deployment shapes plus both-rejected, using stub servers that record the verbs received — asserting not just success but that the client converges on PUT and only falls back when forced. Also covers the 5xx no-fallback rule and path escaping. Verified end to end against Conductor OSS built from main: pause sets "paused": true, resume clears it, both exit 0. Previously both returned 405. Fixes #101 Co-Authored-By: Claude Opus 5 (1M context) --- cmd/scheduler.go | 56 ++++++++++++-- cmd/scheduler_verb_test.go | 150 +++++++++++++++++++++++++++++++++++++ internal/settings.go | 8 +- 3 files changed, 205 insertions(+), 9 deletions(-) create mode 100644 cmd/scheduler_verb_test.go diff --git a/cmd/scheduler.go b/cmd/scheduler.go index 182a7a6..6a424ef 100644 --- a/cmd/scheduler.go +++ b/cmd/scheduler.go @@ -11,7 +11,6 @@ * specific language governing permissions and limitations under the License. */ - package cmd import ( @@ -20,11 +19,12 @@ import ( "errors" "fmt" "github.com/antihax/optional" + "github.com/conductor-oss/conductor-cli/internal" "github.com/conductor-sdk/conductor-go/sdk/client" "github.com/conductor-sdk/conductor-go/sdk/model" - "github.com/conductor-oss/conductor-cli/internal" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" + neturl "net/url" "os" "strings" "text/tabwriter" @@ -239,15 +239,57 @@ func deleteSchedule(cmd *cobra.Command, args []string) error { return nil } +// setSchedulePaused pauses or resumes a single schedule, trying PUT before GET. +// +// The accepted verb differs by deployment: +// +// OSS Conductor PUT only (GET returns 405) +// Orkes Conductor >= 2026-07-14 PUT or GET +// Orkes Conductor < 2026-07-14 GET only (PUT returns 405) +// +// PUT is the RESTful verb and the only one upstream OSS accepts, so it is tried +// first; GET is attempted only if PUT is rejected with a 4xx. That covers all three +// deployments and converges on PUT as older Orkes builds age out, at which point the +// fallback can be deleted. +// +// The fallback is deliberately limited to 4xx. A 5xx or a transport error is +// returned as-is: retrying those with a different verb would mask a real fault and +// report a misleading "method not supported" instead. +// +// This bypasses SchedulerClient.PauseSchedule/ResumeSchedule, which hardcode GET +// (conductor-go v1.8.0, api_scheduler_resource.go:188 and :255). +func setSchedulePaused(ctx context.Context, name, action string) error { + api := internal.GetAPIClient() + path := fmt.Sprintf("/scheduler/schedules/%s/%s", neturl.PathEscape(name), action) + + var result interface{} + _, err := api.Put(ctx, path, nil, &result) + if err == nil { + return nil + } + + var swaggerErr client.GenericSwaggerError + if !errors.As(err, &swaggerErr) { + return err + } + if code := swaggerErr.StatusCode(); code < 400 || code >= 500 { + return err + } + + // Legacy Orkes deployments expose these endpoints as GET. + if _, getErr := api.Get(ctx, path, nil, &result); getErr != nil { + return getErr + } + return nil +} + func pauseSchedule(cmd *cobra.Command, args []string) error { - schedulerClient := internal.GetSchedulerClient() if len(args) == 0 { return cmd.Usage() } for i := 0; i < len(args); i++ { - _, _, err := schedulerClient.PauseSchedule(context.Background(), args[i]) - if err != nil { + if err := setSchedulePaused(context.Background(), args[i], "pause"); err != nil { return parseSchedulerAPIError(err, fmt.Sprintf("Failed to pause schedule '%s'", args[i])) } } @@ -255,14 +297,12 @@ func pauseSchedule(cmd *cobra.Command, args []string) error { } func resumeSchedule(cmd *cobra.Command, args []string) error { - schedulerClient := internal.GetSchedulerClient() if len(args) == 0 { return cmd.Usage() } for i := 0; i < len(args); i++ { - _, _, err := schedulerClient.ResumeSchedule(context.Background(), args[i]) - if err != nil { + if err := setSchedulePaused(context.Background(), args[i], "resume"); err != nil { return parseSchedulerAPIError(err, fmt.Sprintf("Failed to resume schedule '%s'", args[i])) } } diff --git a/cmd/scheduler_verb_test.go b/cmd/scheduler_verb_test.go new file mode 100644 index 0000000..c44bdf9 --- /dev/null +++ b/cmd/scheduler_verb_test.go @@ -0,0 +1,150 @@ +/* + * Copyright 2026 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package cmd + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/conductor-sdk/conductor-go/sdk/client" + "github.com/conductor-sdk/conductor-go/sdk/settings" + + "github.com/conductor-oss/conductor-cli/internal" +) + +// The three deployment shapes setSchedulePaused has to satisfy cannot be covered by +// any single live server, so they are modelled here as stub handlers. Each records +// the verbs it received so the test can assert not just success but that the client +// converged on PUT and only fell back when it had to. + +type verbRecorder struct { + methods []string +} + +func (v *verbRecorder) handler(allow map[string]int) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + v.methods = append(v.methods, r.Method) + if code, ok := allow[r.Method]; ok { + w.WriteHeader(code) + return + } + w.WriteHeader(http.StatusMethodNotAllowed) + } +} + +func withStubServer(t *testing.T, h http.Handler) { + t.Helper() + srv := httptest.NewServer(h) + t.Cleanup(srv.Close) + internal.SetAPIClient(client.NewAPIClient( + settings.NewAuthenticationSettings("", ""), + settings.NewHttpSettings(srv.URL+"/api"), + )) +} + +func TestSetSchedulePausedVerbNegotiation(t *testing.T) { + tests := []struct { + name string + allow map[string]int + wantMethods []string + wantErr bool + }{ + { + // Upstream OSS Conductor: PUT only. One request, no fallback needed. + name: "oss accepts put", + allow: map[string]int{http.MethodPut: http.StatusOK}, + wantMethods: []string{http.MethodPut}, + }, + { + // Orkes >= 2026-07-14 accepts both; PUT must win so we converge on it. + name: "orkes accepts both, put is preferred", + allow: map[string]int{ + http.MethodPut: http.StatusOK, + http.MethodGet: http.StatusOK, + }, + wantMethods: []string{http.MethodPut}, + }, + { + // Legacy Orkes: GET only. PUT is rejected 405, then GET succeeds. + name: "legacy orkes falls back to get", + allow: map[string]int{http.MethodGet: http.StatusOK}, + wantMethods: []string{http.MethodPut, http.MethodGet}, + }, + { + // Neither verb works: surface the failure rather than reporting success. + name: "both rejected returns error", + allow: map[string]int{}, + wantMethods: []string{http.MethodPut, http.MethodGet}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rec := &verbRecorder{} + withStubServer(t, rec.handler(tt.allow)) + + err := setSchedulePaused(context.Background(), "probe", "pause") + if tt.wantErr && err == nil { + t.Fatal("expected an error, got nil") + } + if !tt.wantErr && err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(rec.methods) != len(tt.wantMethods) { + t.Fatalf("methods = %v, want %v", rec.methods, tt.wantMethods) + } + for i, m := range tt.wantMethods { + if rec.methods[i] != m { + t.Fatalf("methods = %v, want %v", rec.methods, tt.wantMethods) + } + } + }) + } +} + +// A 5xx must NOT trigger the fallback. Retrying a server fault with a different verb +// would mask it and surface a misleading "method not supported" instead. +func TestSetSchedulePausedDoesNotFallBackOnServerError(t *testing.T) { + rec := &verbRecorder{} + withStubServer(t, rec.handler(map[string]int{ + http.MethodPut: http.StatusInternalServerError, + http.MethodGet: http.StatusOK, + })) + + if err := setSchedulePaused(context.Background(), "probe", "pause"); err == nil { + t.Fatal("expected the 500 to be returned, got nil") + } + if len(rec.methods) != 1 || rec.methods[0] != http.MethodPut { + t.Fatalf("methods = %v, want a single PUT with no GET fallback", rec.methods) + } +} + +// The schedule name is interpolated into the path and must be escaped. +func TestSetSchedulePausedEscapesName(t *testing.T) { + var gotPath string + withStubServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.EscapedPath() + w.WriteHeader(http.StatusOK) + })) + + if err := setSchedulePaused(context.Background(), "needs escaping", "resume"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if want := "/api/scheduler/schedules/needs%20escaping/resume"; gotPath != want { + t.Fatalf("path = %q, want %q", gotPath, want) + } +} diff --git a/internal/settings.go b/internal/settings.go index ffd3289..77d1eac 100644 --- a/internal/settings.go +++ b/internal/settings.go @@ -11,7 +11,6 @@ * specific language governing permissions and limitations under the License. */ - package internal import ( @@ -52,6 +51,13 @@ func GetSchedulerClient() client.SchedulerClient { return client.NewSchedulerClient(apiClient) } +// GetAPIClient returns the shared SDK API client for the rare case where a typed +// SDK client issues the wrong request and the CLI must drive the endpoint itself. +// Prefer the typed clients; reach for this only with a comment explaining why. +func GetAPIClient() *client.APIClient { + return apiClient +} + func GetTaskClient() *client.TaskResourceApiService { return &client.TaskResourceApiService{APIClient: apiClient} } From 67c8300631f9cacc2afed3cb3deb0183e788ba48 Mon Sep 17 00:00:00 2001 From: francisco-orkes Date: Wed, 5 Aug 2026 14:11:23 -0700 Subject: [PATCH 2/2] fix(scheduler): narrow the GET fallback to 405 and trim the comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback from @kowser-orkes on #107. Narrows the fallback trigger from any 4xx to exactly 405. 405 is the only status that means "wrong verb"; other 4xx have distinct causes and retrying them with GET was wrong: 404 the schedule, or the whole scheduler module, is absent. The retry produced a second 404 and reported that, masking the actionable hint added in #86. 401 auth failure. The retry simply repeated the rejection. Confirmed against OSS Conductor: wrong verb returns 405, a missing schedule and a missing endpoint both return 404 — so the previous range genuinely conflated them. `conductor schedule pause does_not_exist` now surfaces the scheduler-module hint again instead of a fallback-induced error. Also trims the doc comment to the three lines that carry information a reader cannot get from the code. Tests gain 404 and 401 cases asserting no fallback is attempted, alongside the existing 405-falls-back and 5xx-does-not cases. Re-verified end to end against Conductor OSS built from main: pause sets "paused": true, resume clears it. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/scheduler.go | 29 +++++------------------------ cmd/scheduler_verb_test.go | 18 +++++++++++++++++- 2 files changed, 22 insertions(+), 25 deletions(-) diff --git a/cmd/scheduler.go b/cmd/scheduler.go index 6a424ef..6c5d659 100644 --- a/cmd/scheduler.go +++ b/cmd/scheduler.go @@ -24,6 +24,7 @@ import ( "github.com/conductor-sdk/conductor-go/sdk/model" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" + "net/http" neturl "net/url" "os" "strings" @@ -239,25 +240,9 @@ func deleteSchedule(cmd *cobra.Command, args []string) error { return nil } -// setSchedulePaused pauses or resumes a single schedule, trying PUT before GET. -// -// The accepted verb differs by deployment: -// -// OSS Conductor PUT only (GET returns 405) -// Orkes Conductor >= 2026-07-14 PUT or GET -// Orkes Conductor < 2026-07-14 GET only (PUT returns 405) -// -// PUT is the RESTful verb and the only one upstream OSS accepts, so it is tried -// first; GET is attempted only if PUT is rejected with a 4xx. That covers all three -// deployments and converges on PUT as older Orkes builds age out, at which point the -// fallback can be deleted. -// -// The fallback is deliberately limited to 4xx. A 5xx or a transport error is -// returned as-is: retrying those with a different verb would mask a real fault and -// report a misleading "method not supported" instead. -// -// This bypasses SchedulerClient.PauseSchedule/ResumeSchedule, which hardcode GET -// (conductor-go v1.8.0, api_scheduler_resource.go:188 and :255). +// setSchedulePaused pauses or resumes a schedule. OSS Conductor accepts only PUT on +// these endpoints, older Orkes deployments only GET, so try PUT and fall back to GET +// on 405. Any other status is returned as-is. func setSchedulePaused(ctx context.Context, name, action string) error { api := internal.GetAPIClient() path := fmt.Sprintf("/scheduler/schedules/%s/%s", neturl.PathEscape(name), action) @@ -269,14 +254,10 @@ func setSchedulePaused(ctx context.Context, name, action string) error { } var swaggerErr client.GenericSwaggerError - if !errors.As(err, &swaggerErr) { - return err - } - if code := swaggerErr.StatusCode(); code < 400 || code >= 500 { + if !errors.As(err, &swaggerErr) || swaggerErr.StatusCode() != http.StatusMethodNotAllowed { return err } - // Legacy Orkes deployments expose these endpoints as GET. if _, getErr := api.Get(ctx, path, nil, &result); getErr != nil { return getErr } diff --git a/cmd/scheduler_verb_test.go b/cmd/scheduler_verb_test.go index c44bdf9..bb30015 100644 --- a/cmd/scheduler_verb_test.go +++ b/cmd/scheduler_verb_test.go @@ -85,11 +85,27 @@ func TestSetSchedulePausedVerbNegotiation(t *testing.T) { }, { // Neither verb works: surface the failure rather than reporting success. - name: "both rejected returns error", + name: "both rejected with 405 returns error", allow: map[string]int{}, wantMethods: []string{http.MethodPut, http.MethodGet}, wantErr: true, }, + { + // 405 is the only status meaning "wrong verb". A 404 means the schedule — + // or the whole scheduler module — is absent, so retrying with GET would + // produce a second 404 and report that instead of the real cause. + name: "404 does not trigger the fallback", + allow: map[string]int{http.MethodPut: http.StatusNotFound}, + wantMethods: []string{http.MethodPut}, + wantErr: true, + }, + { + // Likewise for auth failures: a fallback would just repeat the rejection. + name: "401 does not trigger the fallback", + allow: map[string]int{http.MethodPut: http.StatusUnauthorized}, + wantMethods: []string{http.MethodPut}, + wantErr: true, + }, } for _, tt := range tests {