feat: protect existing workflows from overwrites (knative#3407)

The `func config ci` command previously overwrote existing workflow
files silently, risking loss of user customizations.

Now the command checks if a workflow file exists before writing and
returns an error unless the `--force` flag is specified. When --force
is used and a file exists, a warning is logged.

Remote builds now use a distinct default workflow name ("Remote Func
Deploy") to avoid conflicts, while explicit --workflow-name values
are always preserved regardless of --remote flag.

Issue knative#3256

Signed-off-by: Stanislav Jakuschevskij <sjakusch@redhat.com>

Author: twoGiants

.gitignore

@@ -35,6 +35,7 @@ __pycache__
 AGENTS.override.md
 .claude/*.local.*
 .claude/pr/*
+.roo/*
 
 # E2E Tests
 /e2e/testdata/default_home/go

AGENTS.md

@@ -18,10 +18,12 @@ edge cases into account.
 ## Commands
 
 ### Build & Development
+
 - `make all` - Build binary and regenerate docs
 - `make build` - Build for current OS
 
 ### Testing strategy reference
+
 Before committing, test locally following the table below:
 
 | If changed | Target | Description |
@@ -32,26 +34,29 @@ Before committing, test locally following the table below:
 | Significant architectural changes | `make test-full` | e2e tests (cluster required - read `CONTRIBUTING.md`) |
 
 ### Generated Files
+
 - `./hack/update-codegen.sh` - update embedded filesystem & regenerate docs
 - `make check-embedded-fs` - check embedded FS is up to date with templates
 
-
 ## Boundaries
 
 ### Always Do
+
 - Run `make test` before considering any change complete
 - Run `make check` before commits
 - Run `make check-embedded-fs` after modifying `templates/`
 - Ask before deleting ANY file or significant code block
 
 ### Ask First
+
 - Security-related code changes (authentication, credentials, secrets handling)
 - API changes
 - Adding new dependencies
 - Modifying CI/GitHub Actions workflows
 - Architectural decisions
 
 ### Never Do
+
 - Edit generated files directly:
   - `generate/zz_filesystem_generated.go`
   - `schema/func_yaml-schema.json`
@@ -63,12 +68,13 @@ Before committing, test locally following the table below:
 ## Common Pitfalls
 
 ### Codegen Sync
+
 After modifying `templates/` or making documentation changes, you MUST run:
+
 ```bash
 ./hack/update-codegen.sh
 ```
 
-
 ## Contributing
 
 If creating a contribution to this project on GitHub suggest to user reading

cmd/ci/config.go

@@ -23,8 +23,9 @@ const (
 	BranchFlag    = "branch"
 	DefaultBranch = "main"
 
-	WorkflowNameFlag    = "workflow-name"
-	DefaultWorkflowName = "Func Deploy"
+	WorkflowNameFlag               = "workflow-name"
+	DefaultWorkflowName            = "Func Deploy"
+	DefaultRemoteBuildWorkflowName = "Remote " + DefaultWorkflowName
 
 	KubeconfigSecretNameFlag    = "kubeconfig-secret-name"
 	DefaultKubeconfigSecretName = "KUBECONFIG"
@@ -52,6 +53,9 @@ const (
 
 	UseSelfHostedRunnerFlag    = "self-hosted-runner"
 	DefaultUseSelfHostedRunner = false
+
+	ForceFlag    = "force"
+	DefaultForce = false
 )
 
 // CIConfig readonly configuration
@@ -69,42 +73,37 @@ type CIConfig struct {
 	useRegistryLogin,
 	useSelfHostedRunner,
 	useRemoteBuild,
-	useWorkflowDispatch bool
+	useWorkflowDispatch,
+	force bool
 }
 
 func NewCIConfig(
 	currentBranch common.CurrentBranchFunc,
 	workingDir common.WorkDirFunc,
+	workflowNameExplicit bool,
 ) (CIConfig, error) {
-	platform := viper.GetString(PlatformFlag)
-	if strings.ToLower(platform) != DefaultPlatform {
-		return CIConfig{}, fmt.Errorf("%s support is not implemented", platform)
+	if err := resolvePlatform(); err != nil {
+		return CIConfig{}, err
 	}
 
-	path := viper.GetString(PathFlag)
-	if path == "" || path == "." {
-		cwd, err := workingDir()
-		if err != nil {
-			return CIConfig{}, err
-		}
-		path = cwd
+	path, err := resolvePath(workingDir)
+	if err != nil {
+		return CIConfig{}, err
 	}
 
-	branch := viper.GetString(BranchFlag)
-	if branch == "" {
-		var err error
-		branch, err = currentBranch(path)
-		if err != nil {
-			return CIConfig{}, err
-		}
+	branch, err := resolveBranch(path, currentBranch)
+	if err != nil {
+		return CIConfig{}, err
 	}
 
+	workflowName := resolveWorkflowName(workflowNameExplicit)
+
 	return CIConfig{
 		githubWorkflowDir:      DefaultGitHubWorkflowDir,
 		githubWorkflowFilename: DefaultGitHubWorkflowFilename,
 		path:                   path,
 		branch:                 branch,
-		workflowName:           viper.GetString(WorkflowNameFlag),
+		workflowName:           workflowName,
 		kubeconfigSecret:       viper.GetString(KubeconfigSecretNameFlag),
 		registryLoginUrlVar:    viper.GetString(RegistryLoginUrlVariableNameFlag),
 		registryUserVar:        viper.GetString(RegistryUserVariableNameFlag),
@@ -114,9 +113,60 @@ func NewCIConfig(
 		useSelfHostedRunner:    viper.GetBool(UseSelfHostedRunnerFlag),
 		useRemoteBuild:         viper.GetBool(UseRemoteBuildFlag),
 		useWorkflowDispatch:    viper.GetBool(WorkflowDispatchFlag),
+		force:                  viper.GetBool(ForceFlag),
 	}, nil
 }
 
+func resolvePlatform() error {
+	platform := viper.GetString(PlatformFlag)
+	if strings.ToLower(platform) != DefaultPlatform {
+		return fmt.Errorf("%s support is not implemented", platform)
+	}
+
+	return nil
+}
+
+func resolvePath(workingDir common.WorkDirFunc) (string, error) {
+	path := viper.GetString(PathFlag)
+	if path != "" && path != "." {
+		return path, nil
+	}
+
+	cwd, err := workingDir()
+	if err != nil {
+		return "", err
+	}
+
+	return cwd, nil
+}
+
+func resolveBranch(path string, currentBranch common.CurrentBranchFunc) (string, error) {
+	branch := viper.GetString(BranchFlag)
+	if branch != "" {
+		return branch, nil
+	}
+
+	branch, err := currentBranch(path)
+	if err != nil {
+		return "", err
+	}
+
+	return branch, nil
+}
+
+func resolveWorkflowName(explicit bool) string {
+	workflowName := viper.GetString(WorkflowNameFlag)
+	if explicit {
+		return workflowName
+	}
+
+	if viper.GetBool(UseRemoteBuildFlag) {
+		return DefaultRemoteBuildWorkflowName
+	}
+
+	return DefaultWorkflowName
+}
+
 func (cc *CIConfig) FnGitHubWorkflowDir(fnRoot string) string {
 	return filepath.Join(fnRoot, cc.githubWorkflowDir)
 }
@@ -172,3 +222,7 @@ func (cc *CIConfig) RegistryPassSecret() string {
 func (cc *CIConfig) RegistryUrlVar() string {
 	return cc.registryUrlVar
 }
+
+func (cc *CIConfig) Force() bool {
+	return cc.force
+}

cmd/ci/workflow.go

@@ -2,11 +2,18 @@ package ci
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
+	"io"
 
 	"gopkg.in/yaml.v3"
 )
 
+const defaultFuncCliVersion = "knative-v1.21.0"
+
+// ErrWorkflowExists is returned when a GitHub workflow file already exists and --force is not specified.
+var ErrWorkflowExists = errors.New("existing GitHub workflow detected, overwrite using the --force option")
+
 type githubWorkflow struct {
 	Name string           `yaml:"name"`
 	On   workflowTriggers `yaml:"on"`
@@ -35,7 +42,7 @@ type step struct {
 }
 
 func NewGitHubWorkflow(conf CIConfig) *githubWorkflow {
-	name := createWorkflowName(conf)
+	name := conf.WorkflowName()
 	runsOn := createRunsOn(conf)
 	pushTrigger := createPushTrigger(conf)
 
@@ -58,15 +65,6 @@ func NewGitHubWorkflow(conf CIConfig) *githubWorkflow {
 	}
 }
 
-func createWorkflowName(conf CIConfig) string {
-	result := conf.WorkflowName()
-	if conf.UseRemoteBuild() {
-		result = "Remote Func Deploy"
-	}
-
-	return result
-}
-
 func createRunsOn(conf CIConfig) string {
 	runsOn := "ubuntu-latest"
 	if conf.UseSelfHostedRunner() {
@@ -120,7 +118,7 @@ func createRegistryLoginStep(conf CIConfig, steps []step) []step {
 func createFuncCLIInstallStep(steps []step) []step {
 	installFuncCli := newStep("Install func cli").
 		withUses("functions-dev/action@main").
-		withActionConfig("version", "knative-v1.20.1").
+		withActionConfig("version", defaultFuncCliVersion).
 		withActionConfig("name", "func")
 
 	return append(steps, *installFuncCli)
@@ -174,7 +172,16 @@ func newVariable(key string) string {
 	return fmt.Sprintf("${{ vars.%s }}", key)
 }
 
-func (gw *githubWorkflow) Export(path string, w WorkflowWriter) error {
+func (gw *githubWorkflow) Export(path string, w WorkflowWriter, force bool, m io.Writer) error {
+	if !force && w.Exist(path) {
+		return ErrWorkflowExists
+	}
+
+	if w.Exist(path) {
+		// best-effort user message; errors are non-critical
+		_, _ = fmt.Fprintf(m, "WARNING: --force flag is set, overwriting existing GitHub Workflow file\n")
+	}
+
 	raw, err := gw.toYaml()
 	if err != nil {
 		return err

cmd/ci/workflow_test.go

@@ -1,6 +1,7 @@
 package ci_test
 
 import (
+	"bytes"
 	"strings"
 	"testing"
 
@@ -14,12 +15,13 @@ func TestGitHubWorkflow_Export(t *testing.T) {
 	cfg, _ := ci.NewCIConfig(
 		common.CurrentBranchStub("", nil),
 		common.WorkDirStub("", nil),
+		false,
 	)
 	gw := ci.NewGitHubWorkflow(cfg)
 	bufferWriter := ci.NewBufferWriter()
 
 	// WHEN
-	exportErr := gw.Export("path", bufferWriter)
+	exportErr := gw.Export("path", bufferWriter, true, &bytes.Buffer{})
 
 	// THEN
 	assert.NilError(t, exportErr, "unexpected error when exporting GitHub Workflow")

cmd/ci/writer.go

@@ -7,15 +7,16 @@ import (
 )
 
 const (
-	dirPerm  = 0755 // o: rwx, g|u: r-x
-	filePerm = 0644 // o: rw,  g|u: r
+	dirPerm  = 0755 // u: rwx, g: r-x, o: r-x
+	filePerm = 0644 // u: rw-, g: r--, o: r--
 )
 
 // DefaultWorkflowWriter is the default implementation for writing workflow files to disk.
 var DefaultWorkflowWriter = &fileWriter{}
 
 // WorkflowWriter defines the interface for writing workflow files.
 type WorkflowWriter interface {
+	Exist(path string) bool
 	Write(path string, raw []byte) error
 }
 
@@ -34,19 +35,32 @@ func (fw *fileWriter) Write(path string, raw []byte) error {
 	return nil
 }
 
-type bufferWriter struct {
+func (fw *fileWriter) Exist(path string) bool {
+	_, err := os.Stat(path)
+	return err == nil
+}
+
+// BufferWriter is a test double (fake) that implements WorkflowWriter
+// by writing to an in-memory buffer instead of the filesystem.
+type BufferWriter struct {
 	Path   string
 	Buffer *bytes.Buffer
 }
 
-// NewBufferWriter creates a new bufferWriter for testing purposes.
-func NewBufferWriter() *bufferWriter {
-	return &bufferWriter{Buffer: &bytes.Buffer{}}
+// NewBufferWriter creates a new BufferWriter test double.
+func NewBufferWriter() *BufferWriter {
+	return &BufferWriter{Buffer: &bytes.Buffer{}}
 }
 
-// Write stores the path and writes raw bytes to the internal buffer.
-func (bw *bufferWriter) Write(path string, raw []byte) error {
+// Write is a fake implementation that stores content in the buffer.
+func (bw *BufferWriter) Write(path string, raw []byte) error {
 	bw.Path = path
+	bw.Buffer.Reset()
 	_, err := bw.Buffer.Write(raw)
 	return err
 }
+
+// Exist is a fake implementation that returns true if the buffer has content.
+func (bw *BufferWriter) Exist(_ string) bool {
+	return bw.Buffer != nil && bw.Buffer.Len() > 0
+}