Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 54 additions & 14 deletions gateway/gateway-builder/internal/policyengine/gomod.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,27 @@ import (
"golang.org/x/mod/modfile"
)

// goGetMaxAttempts and goGetRetryBackoff control retries around the 'go get'
// invocation below. Module proxy fetches occasionally fail with transient
// network errors (e.g. an HTTP/2 stream INTERNAL_ERROR from proxy.golang.org)
// that succeed on a bare retry; a timeout or a genuinely missing/invalid
// module will simply fail again on retry at negligible extra cost.
var (
goGetMaxAttempts = 3
goGetRetryBackoff = 2 * time.Second
)

// runGoGet is a seam over exec.CommandContext so tests can stub out the real
// 'go get' invocation.
var runGoGet = func(ctx context.Context, dir, target string) (stderr []byte, err error) {
cmd := exec.CommandContext(ctx, "go", "get", target)
cmd.Dir = dir
var buf bytes.Buffer
cmd.Stderr = &buf
err = cmd.Run()
return buf.Bytes(), err
}

// UpdateGoMod updates go.mod for discovered policies:
// - gomodule entries: runs 'go get' to pin the real module at its resolved version
// - filePath entries: adds a replace directive pointing to the local path
Expand All @@ -54,23 +75,42 @@ func UpdateGoMod(srcDir string, policies []*types.DiscoveredPolicy) error {
}

target := fmt.Sprintf("%s@%s", policy.GoModulePath, policy.GoModuleVersion)
slog.Debug("running go get for remote policy",
"policy", policy.Name,
"target", target)

ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
cmd := exec.CommandContext(ctx, "go", "get", target)
cmd.Dir = srcDir
var stderr bytes.Buffer
cmd.Stderr = &stderr
if err := cmd.Run(); err != nil {

var lastErr error
var lastStderr []byte
for attempt := 1; attempt <= goGetMaxAttempts; attempt++ {
slog.Debug("running go get for remote policy",
"policy", policy.Name,
"target", target,
"attempt", attempt)

ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
stderr, runErr := runGoGet(ctx, srcDir, target)
timedOut := errors.Is(ctx.Err(), context.DeadlineExceeded)
cancel()
if errors.Is(ctx.Err(), context.DeadlineExceeded) {
return fmt.Errorf("go get timed out after 5min for %s in %s: %w; stderr: %s", target, srcDir, err, stderr.String())

if runErr == nil {
lastErr = nil
break
}
if timedOut {
return fmt.Errorf("go get timed out after 5min for %s in %s: %w; stderr: %s", target, srcDir, runErr, stderr)
}
return fmt.Errorf("go get failed for %s in %s: %w; stderr: %s", target, srcDir, err, stderr.String())

lastErr, lastStderr = runErr, stderr
if attempt < goGetMaxAttempts {
slog.Warn("go get failed, retrying",
"policy", policy.Name,
"target", target,
"attempt", attempt,
"maxAttempts", goGetMaxAttempts,
"err", runErr)
time.Sleep(goGetRetryBackoff * time.Duration(attempt))
}
}
if lastErr != nil {
return fmt.Errorf("go get failed for %s in %s after %d attempts: %w; stderr: %s", target, srcDir, goGetMaxAttempts, lastErr, lastStderr)
}
cancel()
}

// Second pass: add replace directives for filePath (local) entries
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@
package policyengine

import (
"context"
"errors"
"os"
"path/filepath"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -551,6 +554,10 @@ func TestUpdateGoMod_RemotePolicy_InvalidModule(t *testing.T) {
tmpDir := t.TempDir()
testutils.WritePolicyEngineGoMod(t, tmpDir)

// A nonexistent module fails deterministically, not transiently — pin to a
// single attempt so this test isn't slowed down by retry/backoff.
withSingleGoGetAttempt(t)

// Create a remote policy (IsFilePathEntry: false) with invalid module
policies := []*types.DiscoveredPolicy{
testutils.NewRemoteDiscoveredPolicy("ratelimit", "v1.0.0", "github.com/nonexistent-org-12345/nonexistent-module", "v1.0.0"),
Expand Down Expand Up @@ -625,6 +632,10 @@ func TestUpdateGoMod_OnlyRemotePolicies_AllFail(t *testing.T) {
tmpDir := t.TempDir()
testutils.WritePolicyEngineGoMod(t, tmpDir)

// A nonexistent module fails deterministically, not transiently — pin to a
// single attempt so this test isn't slowed down by retry/backoff.
withSingleGoGetAttempt(t)

// Only remote policies, all with invalid modules
policies := []*types.DiscoveredPolicy{
testutils.NewRemoteDiscoveredPolicy("remote-policy", "v1.0.0", "github.com/definitely-not-real-org/fake-module", "v1.0.0"),
Expand All @@ -634,3 +645,74 @@ func TestUpdateGoMod_OnlyRemotePolicies_AllFail(t *testing.T) {
require.Error(t, err)
assert.Contains(t, err.Error(), "go get failed")
}

// withSingleGoGetAttempt temporarily pins goGetMaxAttempts to 1, restoring the
// original value on test cleanup. Use for tests exercising a deterministic
// (non-transient) 'go get' failure, so they aren't slowed by retry/backoff.
func withSingleGoGetAttempt(t *testing.T) {
t.Helper()
original := goGetMaxAttempts
goGetMaxAttempts = 1
t.Cleanup(func() { goGetMaxAttempts = original })
}

// withStubbedGoGet replaces runGoGet with a stub for the duration of the
// test, restoring the original implementation on cleanup. The stub receives
// the 1-indexed attempt number so tests can vary behavior across retries. It
// also shrinks the retry backoff so retry tests run fast, and returns a
// pointer to the running call count.
func withStubbedGoGet(t *testing.T, stub func(attempt int, ctx context.Context, dir, target string) ([]byte, error)) *int {
t.Helper()
callCount := 0
originalRunGoGet := runGoGet
originalBackoff := goGetRetryBackoff
runGoGet = func(ctx context.Context, dir, target string) ([]byte, error) {
callCount++
return stub(callCount, ctx, dir, target)
}
goGetRetryBackoff = time.Millisecond
t.Cleanup(func() {
runGoGet = originalRunGoGet
goGetRetryBackoff = originalBackoff
})
return &callCount
}

func TestUpdateGoMod_RemotePolicy_RetriesTransientFailureThenSucceeds(t *testing.T) {
tmpDir := t.TempDir()
testutils.WritePolicyEngineGoMod(t, tmpDir)

callCount := withStubbedGoGet(t, func(attempt int, ctx context.Context, dir, target string) ([]byte, error) {
if attempt < 3 {
return []byte("stream error: stream ID 1; INTERNAL_ERROR"), errors.New("exit status 1")
}
return nil, nil
})

policies := []*types.DiscoveredPolicy{
testutils.NewRemoteDiscoveredPolicy("semantic-cache", "v1.0.1", "github.com/wso2/gateway-controllers/policies/semantic-cache", "v1.0.1"),
}

err := UpdateGoMod(tmpDir, policies)
require.NoError(t, err)
assert.Equal(t, 3, *callCount, "expected 2 failed attempts followed by 1 successful attempt")
}

func TestUpdateGoMod_RemotePolicy_ExhaustsRetriesOnPersistentFailure(t *testing.T) {
tmpDir := t.TempDir()
testutils.WritePolicyEngineGoMod(t, tmpDir)

callCount := withStubbedGoGet(t, func(attempt int, ctx context.Context, dir, target string) ([]byte, error) {
return []byte("module not found"), errors.New("exit status 1")
})

policies := []*types.DiscoveredPolicy{
testutils.NewRemoteDiscoveredPolicy("ratelimit", "v1.0.0", "github.com/example/policies/ratelimit", "v1.0.0"),
}

err := UpdateGoMod(tmpDir, policies)
require.Error(t, err)
assert.Contains(t, err.Error(), "go get failed")
assert.Contains(t, err.Error(), "after 3 attempts")
assert.Equal(t, goGetMaxAttempts, *callCount)
}
6 changes: 6 additions & 0 deletions platform-api/internal/handler/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ func (h *APIHandler) CreateAPI(w http.ResponseWriter, r *http.Request) error {
return apperror.ValidationFailed.Wrap(err, "Subscription plan not found or not active").
WithLogMessage(fmt.Sprintf("subscription plan not found or not active in org %s", orgId))
}
if errors.Is(err, constants.ErrSecretRefMissing) {
return apperror.ValidationFailed.Wrap(err, "One or more referenced secrets do not exist")
}
return apperror.Internal.Wrap(err).
WithLogMessage(fmt.Sprintf("failed to create API in org %s", orgId))
}
Expand Down Expand Up @@ -268,6 +271,9 @@ func (h *APIHandler) UpdateAPI(w http.ResponseWriter, r *http.Request) error {
return apperror.ValidationFailed.Wrap(err, "Subscription plan not found or not active").
WithLogMessage(fmt.Sprintf("subscription plan not found or not active for API %s in org %s", apiId, orgId))
}
if errors.Is(err, constants.ErrSecretRefMissing) {
return apperror.ValidationFailed.Wrap(err, "One or more referenced secrets do not exist")
}
return apperror.Internal.Wrap(err).
WithLogMessage(fmt.Sprintf("failed to update API %s in org %s", apiId, orgId))
}
Expand Down
4 changes: 4 additions & 0 deletions platform-api/internal/handler/llm.go
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,8 @@ func (h *LLMHandler) CreateLLMProxy(w http.ResponseWriter, r *http.Request) erro
return apperror.ProjectNotFound.Wrap(err)
case errors.Is(err, constants.ErrInvalidPolicyVersion):
return apperror.ValidationFailed.Wrap(err, "Invalid policy version format")
case errors.Is(err, constants.ErrSecretRefMissing):
return apperror.ValidationFailed.Wrap(err, "One or more referenced secrets do not exist")
case errors.Is(err, constants.ErrInvalidInput):
return apperror.ValidationFailed.Wrap(err, "Invalid input")
default:
Expand Down Expand Up @@ -712,6 +714,8 @@ func (h *LLMHandler) UpdateLLMProxy(w http.ResponseWriter, r *http.Request) erro
return apperror.ValidationFailed.Wrap(err, "The id is immutable and cannot be changed")
case errors.Is(err, constants.ErrInvalidPolicyVersion):
return apperror.ValidationFailed.Wrap(err, "Invalid policy version format")
case errors.Is(err, constants.ErrSecretRefMissing):
return apperror.ValidationFailed.Wrap(err, "One or more referenced secrets do not exist")
case errors.Is(err, constants.ErrInvalidInput):
return apperror.ValidationFailed.Wrap(err, "Invalid input")
default:
Expand Down
2 changes: 2 additions & 0 deletions platform-api/internal/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,9 @@ func StartPlatformAPIServer(cfg *config.Server, slogger *slog.Logger) (*Server,
mcpProxyDeploymentHandler := handler.NewMCPProxyDeploymentHandler(mcpDeploymentService, identityService, slogger)
// Wire secret placeholder validation into dependent services
llmProviderService.SetSecretService(secretService)
llmProxyService.SetSecretService(secretService)
mcpProxyService.WithSecretService(secretService)
apiService.SetSecretService(secretService)
secretHandler := handler.NewSecretHandler(secretService, identityService, slogger)
// Start deployment timeout background job
timeoutConfig := service.DeploymentTimeoutConfig{
Expand Down
97 changes: 96 additions & 1 deletion platform-api/internal/service/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type APIService struct {
subscriptionPlanRepo repository.SubscriptionPlanRepository
customPolicyRepo repository.CustomPolicyRepository
gatewayEventsService *GatewayEventsService
secretService *SecretService
apiUtil *utils.APIUtil
slogger *slog.Logger
auditRepo repository.AuditRepository
Expand Down Expand Up @@ -75,6 +76,13 @@ func NewAPIService(apiRepo repository.APIRepository, projectRepo repository.Proj
}
}

// SetSecretService injects the SecretService used to validate {{ secret "..." }}
// placeholders on Create/Update. Called after both services are constructed to
// avoid circular dependency.
func (s *APIService) SetSecretService(ss *SecretService) {
s.secretService = ss
}

// resolveRESTAPIIdentity replaces resp's createdBy/updatedBy UUIDs with the
// raw external identity (or constants.DeletedUser), in place.
func (s *APIService) resolveRESTAPIIdentity(resp *api.RESTAPI) error {
Expand All @@ -94,6 +102,20 @@ func (s *APIService) CreateAPI(req *api.CreateRESTAPIRequest, orgUUID, createdBy
return nil, err
}

// Validate {{ secret "..." }} placeholders anywhere in the request — the
// gateway-controller's template engine resolves placeholders generically
// across the whole artifact (upstream auth and policies alike), so
// validation must cover the same surface.
if s.secretService != nil {
configJSON, err := marshalUpstreamForValidation(req)
if err != nil {
return nil, fmt.Errorf("failed to marshal request for secret validation: %w", err)
}
if err := s.secretService.ValidateSecretRefs(orgUUID, configJSON); err != nil {
return nil, err
}
}

projectHandle := strings.TrimSpace(req.ProjectId)
// Check if project exists
project, err := s.projectRepo.GetProjectByHandleAndOrgID(projectHandle, orgUUID)
Expand Down Expand Up @@ -301,6 +323,18 @@ func (s *APIService) UpdateAPI(apiUUID string, req *api.RESTAPI, orgUUID, update
return nil, constants.ErrAPINotFound
}

// Validate {{ secret "..." }} placeholders anywhere in the request — see
// CreateAPI for why this covers the whole request, not just upstream.
if s.secretService != nil {
configJSON, err := marshalUpstreamForValidation(req)
if err != nil {
return nil, fmt.Errorf("failed to marshal request for secret validation: %w", err)
}
if err := s.secretService.ValidateSecretRefs(orgUUID, configJSON); err != nil {
return nil, err
}
}

// Apply updates using shared helper
updatedAPI, err := s.applyAPIUpdates(existingAPIModel, req, orgUUID)
if err != nil {
Expand Down Expand Up @@ -331,6 +365,26 @@ func (s *APIService) UpdateAPI(apiUUID string, req *api.RESTAPI, orgUUID, update
return nil, err
}

// Best-effort: delete secrets rotated away from on Main/Sandbox auth. Must
// run after the update above persists the new reference, so the in-use
// check no longer sees this API pointing at the old handle.
if s.secretService != nil {
s.secretService.cleanupRotatedSecret(
orgUUID,
mainUpstreamAuthValue(&existingAPIModel.Configuration.Upstream),
mainUpstreamAuthValue(&updatedAPIModel.Configuration.Upstream),
updatedBy,
s.slogger,
)
s.secretService.cleanupRotatedSecret(
orgUUID,
sandboxUpstreamAuthValue(&existingAPIModel.Configuration.Upstream),
sandboxUpstreamAuthValue(&updatedAPIModel.Configuration.Upstream),
updatedBy,
s.slogger,
)
}

s.refreshCustomPolicyUsages(apiUUID, orgUUID, updatedAPIModel)

_ = s.auditRepo.Record("UPDATE", apiUUID, "rest_api", orgUUID, updatedBy)
Expand Down Expand Up @@ -735,12 +789,53 @@ func (s *APIService) applyAPIUpdates(existingAPIModel *model.API, req *api.RESTA
existingAPI.Policies = req.Policies
existingAPI.SubscriptionPlans = req.SubscriptionPlans
if !s.isEmptyUpstream(req.Upstream) {
existingAPI.Upstream = req.Upstream
// isEmptyUpstream only looks at Url/Ref, so a request that updates the
// URL but omits auth (routine — auth.value is redacted on GET, so a
// naive read-modify-write round-trip never has it to resend) must not
// wipe the stored credential. Preserve auth from the existing model
// (unredacted) whenever the incoming value is empty.
existingAPI.Upstream = preserveUpstreamAuthOnAPIUpdate(existingAPIModel.Configuration.Upstream, req.Upstream)
}

return existingAPI, nil
}

// preserveUpstreamAuthOnAPIUpdate merges updated's Main/Sandbox auth onto
// existing's stored (unredacted) values whenever updated's auth value is
// empty, then returns updated with those merged auth blocks. Non-auth fields
// (Url, Ref) are taken from updated as-is.
func preserveUpstreamAuthOnAPIUpdate(existing model.UpstreamConfig, updated api.Upstream) api.Upstream {
updated.Main.Auth = preserveAPIUpstreamAuth(existing.Main, updated.Main.Auth)
if updated.Sandbox != nil {
updated.Sandbox.Auth = preserveAPIUpstreamAuth(existing.Sandbox, updated.Sandbox.Auth)
}
return updated
}

// preserveAPIUpstreamAuth backfills updated's Value from existing's stored
// value when updated is nil or has an empty value. The stored value is only
// reused when updated's Type matches existing's (or leaves Type unspecified);
// an explicit Type change means the old secret is for a different auth
// scheme and must not be silently reattached.
func preserveAPIUpstreamAuth(existing *model.UpstreamEndpoint, updated *api.UpstreamAuth) *api.UpstreamAuth {
if existing == nil || existing.Auth == nil || existing.Auth.Value == "" {
return updated
}
if updated == nil {
authType := api.UpstreamAuthType(existing.Auth.Type)
return &api.UpstreamAuth{
Type: &authType,
Header: utils.StringPtrIfNotEmpty(existing.Auth.Header),
Value: utils.StringPtrIfNotEmpty(existing.Auth.Value),
}
}
typeChanged := updated.Type != nil && string(*updated.Type) != existing.Auth.Type
if !typeChanged && (updated.Value == nil || *updated.Value == "") {
updated.Value = utils.StringPtrIfNotEmpty(existing.Auth.Value)
}
return updated
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
// validateUpdateAPIRequest checks the validity of the update API request
func (s *APIService) validateUpdateAPIRequest(existingAPIModel *model.API, req *api.RESTAPI, orgUUID string) error {
if req.DisplayName != "" {
Expand Down
Loading
Loading