feat: monolithic env var editing (bulk save, no per-var CRUD)
All checks were successful
Check / check (pull_request) Successful in 4s
All checks were successful
Check / check (pull_request) Successful in 4s
Replace individual env var add/edit/delete with a single bulk save
endpoint. The UI now shows a textarea with KEY=VALUE lines. On save,
all existing env vars are deleted and the full submitted set is
inserted.
- Replace HandleEnvVarAdd, HandleEnvVarEdit, HandleEnvVarDelete with
HandleEnvVarSave
- Collapse 3 routes into single POST /apps/{id}/env
- Template uses textarea instead of per-row edit/delete forms
- No individual env var IDs exposed in the UI
- Extract parseEnvPairs helper to keep cyclomatic complexity low
- Use strings.SplitSeq per modernize linter
- Update tests for new bulk save behavior
closes #156
closes #163
This commit is contained in:
@@ -903,51 +903,98 @@ func (h *Handlers) addKeyValueToApp(
|
||||
http.Redirect(writer, request, "/apps/"+application.ID, http.StatusSeeOther)
|
||||
}
|
||||
|
||||
// HandleEnvVarAdd handles adding an environment variable.
|
||||
func (h *Handlers) HandleEnvVarAdd() http.HandlerFunc {
|
||||
// HandleEnvVarSave handles bulk saving of all environment variables.
|
||||
// It deletes all existing env vars for the app and inserts the full
|
||||
// submitted set (monolithic list approach).
|
||||
func (h *Handlers) HandleEnvVarSave() http.HandlerFunc {
|
||||
return func(writer http.ResponseWriter, request *http.Request) {
|
||||
h.addKeyValueToApp(
|
||||
appID := chi.URLParam(request, "id")
|
||||
|
||||
application, findErr := models.FindApp(request.Context(), h.db, appID)
|
||||
if findErr != nil || application == nil {
|
||||
http.NotFound(writer, request)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
parseErr := request.ParseForm()
|
||||
if parseErr != nil {
|
||||
http.Error(writer, "Bad Request", http.StatusBadRequest)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
pairs := parseEnvPairs(request.FormValue("env_vars"))
|
||||
ctx := request.Context()
|
||||
|
||||
// Delete all existing env vars for this app
|
||||
deleteErr := models.DeleteEnvVarsByAppID(ctx, h.db, application.ID)
|
||||
if deleteErr != nil {
|
||||
h.log.Error("failed to delete env vars", "error", deleteErr)
|
||||
http.Error(
|
||||
writer,
|
||||
"Internal Server Error",
|
||||
http.StatusInternalServerError,
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
// Insert the full new set
|
||||
for _, p := range pairs {
|
||||
envVar := models.NewEnvVar(h.db)
|
||||
envVar.AppID = application.ID
|
||||
envVar.Key = p.key
|
||||
envVar.Value = p.value
|
||||
|
||||
saveErr := envVar.Save(ctx)
|
||||
if saveErr != nil {
|
||||
h.log.Error(
|
||||
"failed to save env var",
|
||||
"key", p.key,
|
||||
"error", saveErr,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
http.Redirect(
|
||||
writer,
|
||||
request,
|
||||
func(ctx context.Context, application *models.App, key, value string) error {
|
||||
envVar := models.NewEnvVar(h.db)
|
||||
envVar.AppID = application.ID
|
||||
envVar.Key = key
|
||||
envVar.Value = value
|
||||
|
||||
return envVar.Save(ctx)
|
||||
},
|
||||
"/apps/"+appID+"?success=env-updated",
|
||||
http.StatusSeeOther,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// HandleEnvVarDelete handles deleting an environment variable.
|
||||
func (h *Handlers) HandleEnvVarDelete() http.HandlerFunc {
|
||||
return func(writer http.ResponseWriter, request *http.Request) {
|
||||
appID := chi.URLParam(request, "id")
|
||||
envVarIDStr := chi.URLParam(request, "varID")
|
||||
// envPair holds a parsed key-value pair from the env vars textarea.
|
||||
type envPair struct {
|
||||
key string
|
||||
value string
|
||||
}
|
||||
|
||||
envVarID, parseErr := strconv.ParseInt(envVarIDStr, 10, 64)
|
||||
if parseErr != nil {
|
||||
http.NotFound(writer, request)
|
||||
// parseEnvPairs parses KEY=VALUE lines from a textarea string.
|
||||
// Blank lines and lines starting with # are skipped.
|
||||
func parseEnvPairs(text string) []envPair {
|
||||
var pairs []envPair
|
||||
|
||||
return
|
||||
for line := range strings.SplitSeq(text, "\n") {
|
||||
line = strings.TrimSpace(line)
|
||||
if line == "" || strings.HasPrefix(line, "#") {
|
||||
continue
|
||||
}
|
||||
|
||||
envVar, findErr := models.FindEnvVar(request.Context(), h.db, envVarID)
|
||||
if findErr != nil || envVar == nil || envVar.AppID != appID {
|
||||
http.NotFound(writer, request)
|
||||
|
||||
return
|
||||
key, value, ok := strings.Cut(line, "=")
|
||||
if !ok || strings.TrimSpace(key) == "" {
|
||||
continue
|
||||
}
|
||||
|
||||
deleteErr := envVar.Delete(request.Context())
|
||||
if deleteErr != nil {
|
||||
h.log.Error("failed to delete env var", "error", deleteErr)
|
||||
}
|
||||
|
||||
http.Redirect(writer, request, "/apps/"+appID, http.StatusSeeOther)
|
||||
pairs = append(pairs, envPair{
|
||||
key: strings.TrimSpace(key),
|
||||
value: value,
|
||||
})
|
||||
}
|
||||
|
||||
return pairs
|
||||
}
|
||||
|
||||
// HandleLabelAdd handles adding a label.
|
||||
@@ -1205,59 +1252,6 @@ func ValidateVolumePath(p string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// HandleEnvVarEdit handles editing an existing environment variable.
|
||||
func (h *Handlers) HandleEnvVarEdit() http.HandlerFunc {
|
||||
return func(writer http.ResponseWriter, request *http.Request) {
|
||||
appID := chi.URLParam(request, "id")
|
||||
envVarIDStr := chi.URLParam(request, "varID")
|
||||
|
||||
envVarID, parseErr := strconv.ParseInt(envVarIDStr, 10, 64)
|
||||
if parseErr != nil {
|
||||
http.NotFound(writer, request)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
envVar, findErr := models.FindEnvVar(request.Context(), h.db, envVarID)
|
||||
if findErr != nil || envVar == nil || envVar.AppID != appID {
|
||||
http.NotFound(writer, request)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
formErr := request.ParseForm()
|
||||
if formErr != nil {
|
||||
http.Error(writer, "Bad Request", http.StatusBadRequest)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
key := request.FormValue("key")
|
||||
value := request.FormValue("value")
|
||||
|
||||
if key == "" || value == "" {
|
||||
http.Redirect(writer, request, "/apps/"+appID, http.StatusSeeOther)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
envVar.Key = key
|
||||
envVar.Value = value
|
||||
|
||||
saveErr := envVar.Save(request.Context())
|
||||
if saveErr != nil {
|
||||
h.log.Error("failed to update env var", "error", saveErr)
|
||||
}
|
||||
|
||||
http.Redirect(
|
||||
writer,
|
||||
request,
|
||||
"/apps/"+appID+"?success=env-updated",
|
||||
http.StatusSeeOther,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// HandleLabelEdit handles editing an existing label.
|
||||
func (h *Handlers) HandleLabelEdit() http.HandlerFunc {
|
||||
return func(writer http.ResponseWriter, request *http.Request) {
|
||||
|
||||
@@ -560,45 +560,89 @@ func testOwnershipVerification(t *testing.T, cfg ownedResourceTestConfig) {
|
||||
cfg.verifyFn(t, testCtx, resourceID)
|
||||
}
|
||||
|
||||
// TestDeleteEnvVarOwnershipVerification tests that deleting an env var
|
||||
// via another app's URL path returns 404 (IDOR prevention).
|
||||
func TestDeleteEnvVarOwnershipVerification(t *testing.T) { //nolint:dupl // intentionally similar IDOR test pattern
|
||||
// TestHandleEnvVarSaveBulk tests that HandleEnvVarSave replaces all env vars
|
||||
// for an app with the submitted set (monolithic delete-all + insert-all).
|
||||
func TestHandleEnvVarSaveBulk(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
testOwnershipVerification(t, ownedResourceTestConfig{
|
||||
appPrefix1: "envvar-owner-app",
|
||||
appPrefix2: "envvar-other-app",
|
||||
createFn: func(t *testing.T, tc *testContext, ownerApp *models.App) int64 {
|
||||
t.Helper()
|
||||
testCtx := setupTestHandlers(t)
|
||||
createdApp := createTestApp(t, testCtx, "envvar-bulk-app")
|
||||
|
||||
envVar := models.NewEnvVar(tc.database)
|
||||
envVar.AppID = ownerApp.ID
|
||||
envVar.Key = "SECRET"
|
||||
envVar.Value = "hunter2"
|
||||
require.NoError(t, envVar.Save(context.Background()))
|
||||
// Create some pre-existing env vars
|
||||
for _, kv := range [][2]string{{"OLD_KEY", "old_value"}, {"REMOVE_ME", "gone"}} {
|
||||
ev := models.NewEnvVar(testCtx.database)
|
||||
ev.AppID = createdApp.ID
|
||||
ev.Key = kv[0]
|
||||
ev.Value = kv[1]
|
||||
require.NoError(t, ev.Save(context.Background()))
|
||||
}
|
||||
|
||||
return envVar.ID
|
||||
},
|
||||
deletePath: func(appID string, resourceID int64) string {
|
||||
return "/apps/" + appID + "/env/" + strconv.FormatInt(resourceID, 10) + "/delete"
|
||||
},
|
||||
chiParams: func(appID string, resourceID int64) map[string]string {
|
||||
return map[string]string{"id": appID, "varID": strconv.FormatInt(resourceID, 10)}
|
||||
},
|
||||
handler: func(h *handlers.Handlers) http.HandlerFunc { return h.HandleEnvVarDelete() },
|
||||
verifyFn: func(t *testing.T, tc *testContext, resourceID int64) {
|
||||
t.Helper()
|
||||
// Submit a new set via textarea
|
||||
form := url.Values{}
|
||||
form.Set("env_vars", "NEW_KEY=new_value\nANOTHER=42\n# comment line\n")
|
||||
|
||||
found, findErr := models.FindEnvVar(context.Background(), tc.database, resourceID)
|
||||
require.NoError(t, findErr)
|
||||
assert.NotNil(t, found, "env var should still exist after IDOR attempt")
|
||||
},
|
||||
})
|
||||
r := chi.NewRouter()
|
||||
r.Post("/apps/{id}/env", testCtx.handlers.HandleEnvVarSave())
|
||||
|
||||
request := httptest.NewRequest(
|
||||
http.MethodPost,
|
||||
"/apps/"+createdApp.ID+"/env",
|
||||
strings.NewReader(form.Encode()),
|
||||
)
|
||||
request.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
|
||||
recorder := httptest.NewRecorder()
|
||||
r.ServeHTTP(recorder, request)
|
||||
|
||||
assert.Equal(t, http.StatusSeeOther, recorder.Code)
|
||||
|
||||
// Verify old env vars are gone and new ones exist
|
||||
envVars, err := models.FindEnvVarsByAppID(
|
||||
context.Background(), testCtx.database, createdApp.ID,
|
||||
)
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, envVars, 2)
|
||||
|
||||
keys := make(map[string]string)
|
||||
for _, ev := range envVars {
|
||||
keys[ev.Key] = ev.Value
|
||||
}
|
||||
|
||||
assert.Equal(t, "new_value", keys["NEW_KEY"])
|
||||
assert.Equal(t, "42", keys["ANOTHER"])
|
||||
assert.Empty(t, keys["OLD_KEY"], "old env vars should be deleted")
|
||||
assert.Empty(t, keys["REMOVE_ME"], "old env vars should be deleted")
|
||||
}
|
||||
|
||||
// TestHandleEnvVarSaveAppNotFound tests that HandleEnvVarSave returns 404
|
||||
// for a non-existent app.
|
||||
func TestHandleEnvVarSaveAppNotFound(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
testCtx := setupTestHandlers(t)
|
||||
|
||||
form := url.Values{}
|
||||
form.Set("env_vars", "KEY=value\n")
|
||||
|
||||
r := chi.NewRouter()
|
||||
r.Post("/apps/{id}/env", testCtx.handlers.HandleEnvVarSave())
|
||||
|
||||
request := httptest.NewRequest(
|
||||
http.MethodPost,
|
||||
"/apps/nonexistent-id/env",
|
||||
strings.NewReader(form.Encode()),
|
||||
)
|
||||
request.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
|
||||
recorder := httptest.NewRecorder()
|
||||
r.ServeHTTP(recorder, request)
|
||||
|
||||
assert.Equal(t, http.StatusNotFound, recorder.Code)
|
||||
}
|
||||
|
||||
// TestDeleteLabelOwnershipVerification tests that deleting a label
|
||||
// via another app's URL path returns 404 (IDOR prevention).
|
||||
func TestDeleteLabelOwnershipVerification(t *testing.T) { //nolint:dupl // intentionally similar IDOR test pattern
|
||||
func TestDeleteLabelOwnershipVerification(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
testOwnershipVerification(t, ownedResourceTestConfig{
|
||||
@@ -714,41 +758,46 @@ func TestDeletePortOwnershipVerification(t *testing.T) {
|
||||
assert.NotNil(t, found, "port should still exist after IDOR attempt")
|
||||
}
|
||||
|
||||
// TestHandleEnvVarDeleteUsesCorrectRouteParam verifies that HandleEnvVarDelete
|
||||
// reads the "varID" chi URL parameter (matching the route definition {varID}),
|
||||
// not a mismatched name like "envID".
|
||||
func TestHandleEnvVarDeleteUsesCorrectRouteParam(t *testing.T) {
|
||||
// TestHandleEnvVarSaveEmptyClears verifies that submitting an empty textarea
|
||||
// deletes all existing env vars for the app.
|
||||
func TestHandleEnvVarSaveEmptyClears(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
testCtx := setupTestHandlers(t)
|
||||
createdApp := createTestApp(t, testCtx, "envvar-clear-app")
|
||||
|
||||
createdApp := createTestApp(t, testCtx, "envdelete-param-app")
|
||||
// Create a pre-existing env var
|
||||
ev := models.NewEnvVar(testCtx.database)
|
||||
ev.AppID = createdApp.ID
|
||||
ev.Key = "DELETE_ME"
|
||||
ev.Value = "gone"
|
||||
require.NoError(t, ev.Save(context.Background()))
|
||||
|
||||
envVar := models.NewEnvVar(testCtx.database)
|
||||
envVar.AppID = createdApp.ID
|
||||
envVar.Key = "DELETE_ME"
|
||||
envVar.Value = "gone"
|
||||
require.NoError(t, envVar.Save(context.Background()))
|
||||
// Submit empty textarea
|
||||
form := url.Values{}
|
||||
form.Set("env_vars", "")
|
||||
|
||||
// Use chi router with the real route pattern to test param name
|
||||
r := chi.NewRouter()
|
||||
r.Post("/apps/{id}/env/{varID}/delete", testCtx.handlers.HandleEnvVarDelete())
|
||||
r.Post("/apps/{id}/env", testCtx.handlers.HandleEnvVarSave())
|
||||
|
||||
request := httptest.NewRequest(
|
||||
http.MethodPost,
|
||||
"/apps/"+createdApp.ID+"/env/"+strconv.FormatInt(envVar.ID, 10)+"/delete",
|
||||
nil,
|
||||
"/apps/"+createdApp.ID+"/env",
|
||||
strings.NewReader(form.Encode()),
|
||||
)
|
||||
recorder := httptest.NewRecorder()
|
||||
request.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
|
||||
recorder := httptest.NewRecorder()
|
||||
r.ServeHTTP(recorder, request)
|
||||
|
||||
assert.Equal(t, http.StatusSeeOther, recorder.Code)
|
||||
|
||||
// Verify the env var was actually deleted
|
||||
found, findErr := models.FindEnvVar(context.Background(), testCtx.database, envVar.ID)
|
||||
require.NoError(t, findErr)
|
||||
assert.Nil(t, found, "env var should be deleted when using correct route param")
|
||||
// Verify all env vars are gone
|
||||
envVars, err := models.FindEnvVarsByAppID(
|
||||
context.Background(), testCtx.database, createdApp.ID,
|
||||
)
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, envVars, "all env vars should be deleted")
|
||||
}
|
||||
|
||||
// TestHandleVolumeAddValidatesPaths verifies that HandleVolumeAdd validates
|
||||
|
||||
Reference in New Issue
Block a user