Skip to content
Merged
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
88 changes: 52 additions & 36 deletions pkg/plugins/golang/v4/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"os"
"path/filepath"
"strings"
"unicode"

"github.com/spf13/pflag"

Expand Down Expand Up @@ -158,57 +157,74 @@ func (p *initSubcommand) PostScaffold() error {
return nil
}

// checkDir will return error if the current directory has files which are not allowed.
// Note that, it is expected that the directory to scaffold the project is cleaned.
// Otherwise, it might face issues to do the scaffold.
// checkDir checks the target directory before scaffolding:
// 1. Returns error if key kubebuilder files already exist (prevents re-initialization)
// 2. Warns if directory is not empty (but allows scaffolding to continue)
func checkDir() error {
// Files scaffolded by 'kubebuilder init' that indicate the directory is already initialized.
// Blocking these prevents accidental re-initialization and file conflicts.
// Note: go.mod and go.sum are NOT blocked because:
// - They may exist in pre-existing Go projects
// - Kubebuilder will overwrite them (machinery.OverwriteFile)
// - Testdata generation creates go.mod before running init
scaffoldedFiles := []string{
"PROJECT", // Kubebuilder project config (key indicator)
"Makefile", // Build automation
filepath.Join("cmd", "main.go"), // Controller manager entry point
}

// Check for existing scaffolded files
for _, file := range scaffoldedFiles {
if _, err := os.Stat(file); err == nil {
return fmt.Errorf("target directory is already initialized. "+
"Found existing kubebuilder file %q. "+
"Please run this command in a new directory or remove existing scaffolded files", file)
}
}

// Check if directory has any other files (warn only)
// Note: We ignore certain files that are expected or safely overwritten:
// - go.mod and go.sum: Users may run `go mod init` before `kubebuilder init`
// - .gitignore and .dockerignore: Safely overwritten by kubebuilder
// - Other dot directories (.git, .vscode, .idea): Not scaffolded by kubebuilder
// However, we DO check .github directory since kubebuilder scaffolds workflows there
var hasFiles bool
err := filepath.Walk(".",
func(path string, info os.FileInfo, err error) error {
if err != nil {
return fmt.Errorf("error walking path %q: %w", path, err)
}
// Allow directory trees starting with '.'
if info.IsDir() && strings.HasPrefix(info.Name(), ".") && info.Name() != "." {
return filepath.SkipDir
}
// Allow files starting with '.'
if strings.HasPrefix(info.Name(), ".") {
return nil
}
// Allow files ending with '.md' extension
if strings.HasSuffix(info.Name(), ".md") && !info.IsDir() {
// Skip the current directory itself
if path == "." {
return nil
}
// Allow capitalized files except PROJECT
isCapitalized := true
for _, l := range info.Name() {
if !unicode.IsUpper(l) {
isCapitalized = false
break
// Skip dot directories EXCEPT .github (which contains scaffolded workflows)
if info.IsDir() && strings.HasPrefix(info.Name(), ".") {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove warnings on dot directories?
As a user, I think I would like to be warn if my GitHub workflow was about to be overridden. Or am I missing something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! 🙏 I tried to improve it, but just a heads-up — the warning might show up quite a bit, since .idea files (and others) can cause false positives. We have a ton of those.

Please let me know if you spot any important cases that we should actually handle.

Examples:

  • .github/workflows/ci.yml⚠️ warning shown
  • .git, .vscode, .idea → ✅ skipped (no warning)
  • Only .gitignore, go.mod, go.sum → ✅ ignored (no warning)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I was missing ! I forgot that the code doesn't check only for kubebuilder files !

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @robinlioret

WDYT?

Could we win your LGTM now to get this one merged?

Copy link

@robinlioret robinlioret Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well. I thinks we should keep the warnings for all the files including the dot files. I feel that skipping them may confuse users.

Imagine the following

  1. The user tries to initialize in a repo with a .GitHub dir
  2. They receive a set of warnings, they assumes that the list is complete.
  3. They accept the warning and get a replaced .github.

If the user doesn't check their git diff, they'll miss the change and could cause issues later on.

This makes sense because nowadays, dot files have very important roles.

Copy link
Member Author

@camilamacedo86 camilamacedo86 Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.github/workflows/ci.yml seems like the only place where this actually makes
sense. For the other directories, raising a warning doesn’t seem useful and
would just create noise. If we warn on everything, we end up training people
to ignore warnings entirely, which isn’t great practice. It’s better to only
warn when it’s meaningful.

Copy link

@robinlioret robinlioret Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. The best option would be to add a preparation step that list the files about to be created by kubebuilder and check against this list. Maybe such a list could be statically created at build time?

For the time being, I don't know if it is worth the effort. So the solution in this PR could be a quick win. Let's merge!

if info.Name() != ".github" {
return filepath.SkipDir
}
}
if isCapitalized && info.Name() != "PROJECT" {
return nil
}
disallowedExtensions := []string{
".go",
".yaml",
".mod",
".sum",
}
// Deny files with .go or .yaml or .mod or .sum extensions
for _, ext := range disallowedExtensions {
if strings.HasSuffix(info.Name(), ext) {
// Skip files that are expected or safely overwritten
ignoredFiles := []string{"go.mod", "go.sum"}
for _, ignored := range ignoredFiles {
if info.Name() == ignored {
return nil
}
}
// Do not allow any other file
return fmt.Errorf("target directory is not empty and contains a disallowed file %q. "+
"files with the following extensions [%s] are not allowed to avoid conflicts with the tooling",
path, strings.Join(disallowedExtensions, ", "))
// Track if any other files/directories exist
hasFiles = true
return nil
})
if err != nil {
return fmt.Errorf("error walking directory: %w", err)
}

// Warn if directory is not empty (but don't block)
if hasFiles {
log.Warn("The target directory is not empty. " +
"Scaffolding may overwrite existing files or cause conflicts. " +
"It is recommended to initialize in an empty directory.")
}

return nil
}