Skip to content

✨ (CLI/Api): Add IfNotExistsAction to machinery for optional file handling #4967

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
14 changes: 14 additions & 0 deletions pkg/machinery/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@ const (
OverwriteFile
)

// IfNotExistsAction determines what to do if a file to be updated does not exist
type IfNotExistsAction int

const (
// ErrorIfNotExist returns an error and stops processing (default behavior)
ErrorIfNotExist IfNotExistsAction = iota

// IgnoreFile skips the file and logs a message if it does not exist
IgnoreFile
)

// File describes a file that will be written
type File struct {
// Path is the file to write
Expand All @@ -40,4 +51,7 @@ type File struct {

// IfExistsAction determines what to do if the file exists
IfExistsAction IfExistsAction

// IfNotExistsAction determines what to do if the file is missing (optional updates only)
IfNotExistsAction IfNotExistsAction
}
5 changes: 5 additions & 0 deletions pkg/machinery/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ type Inserter interface {
GetCodeFragments() CodeFragmentsMap
}

// HasIfNotExistsAction allows a template to define an action if the file is missing
type HasIfNotExistsAction interface {
GetIfNotExistsAction() IfNotExistsAction
}

// HasDomain allows the domain to be used on a template
type HasDomain interface {
// InjectDomain sets the template domain
Expand Down
11 changes: 11 additions & 0 deletions pkg/machinery/mixins.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,3 +153,14 @@ func (m *ResourceMixin) InjectResource(res *resource.Resource) {
m.Resource = res
}
}

// IfNotExistsActionMixin provides file builders with an if-not-exists-action field
type IfNotExistsActionMixin struct {
// IfNotExistsAction determines what to do if the file does not exist
IfNotExistsAction IfNotExistsAction
}

// GetIfNotExistsAction implements Inserter
func (m *IfNotExistsActionMixin) GetIfNotExistsAction() IfNotExistsAction {
return m.IfNotExistsAction
}
22 changes: 15 additions & 7 deletions pkg/machinery/scaffold.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,13 +237,21 @@ func doTemplate(t Template) ([]byte, error) {
func (s Scaffold) updateFileModel(i Inserter, models map[string]*File) error {
m, err := s.loadPreviousModel(i, models)
if err != nil {
// TODO(kubebuilder/issues/4960): Create Machinery implementation to allow defining IfNotExistsAction
// If the file path starts with test/, warn and skip
// Workaround to allow projects be backwards compatible with previous versions
if strings.HasPrefix(i.GetPath(), "test/") {
log.Warn("Skipping missing test file", "file_path", i.GetPath())
log.Warn("The code fragments will not be inserted.")
return nil
if os.IsNotExist(err) {
if withOptionalBehavior, ok := i.(HasIfNotExistsAction); ok {
switch withOptionalBehavior.GetIfNotExistsAction() {
case IgnoreFile:
log.Warn("Skipping missing file", "file", i.GetPath())
log.Warn("The code fragments will not be inserted.")
return nil
case ErrorIfNotExist:
return err
default:
return err
}
}
// If inserter doesn't implement HasIfNotExistsAction, return the original error
Copy link
Preview

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

The comment indicates that if the inserter doesn't implement HasIfNotExistsAction, the original error should be returned, but the code will actually fall through to line 256 which wraps the error in a different format. This inconsistency between comment and implementation could lead to confusion about error handling behavior.

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is this comment okay?
// If HasIfNotExistsAction is not implemented, fall through and return a wrapped error.

return err
}
return fmt.Errorf("failed to load previous model for %s: %w", i.GetPath(), err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type WebhookTest struct {
machinery.MultiGroupMixin
machinery.BoilerplateMixin
machinery.ResourceMixin
machinery.IfNotExistsActionMixin

Force bool

Expand Down Expand Up @@ -78,6 +79,7 @@ func (f *WebhookTest) SetTemplateDefaults() error {
if f.Force {
f.IfExistsAction = machinery.OverwriteFile
}
f.IfNotExistsAction = machinery.IgnoreFile

return nil
}
Expand Down