Just for my own culture, is proning client.ForceOwnership a result of "a single resource should not be managed by multiple operator" logic ?

Updated. Good catcher !!!

I'm not quite sure on how to phrase it, but should we also say "Expect other people to automate your CRs through higher level operators" ?

As in the operator that has generated SSA types would not be the one using them but merely providing them for other operators to import ?

done

Stumbled on this PR while searching for recommendations on SSA for kubebuilder controllers. looks like client.Apply is deprecated. Is this still the way to do SSA?

fixed

@JoelSpeed @alvaroaleman @sbueringer
It would be great if we could not need to add at all here the applyconfiguration since it will only generate the resources when the ac marker is used and a project can have CRDs that are SSA and others that not.

The need to add this does not bring a nice UX.
Could we not change the ssa marker / generator to work as any other marker in the controller-tools?

@JoelSpeed @alvaroaleman @sbueringer

Could you please give a hand on the review of this doc?
Could you please let us know if has any info here that is not accurate or can/should be shaped?

This works, but the nested fallback structure is a little hard to scan. Since the intent is “try these known Makefile formats in order and succeed on the first match,” I think this could be made table-driven.

Something like:

func replaceObjectGenInMakefile(makefilePath string) error {
	replacements := []struct {
		old string
		new string
	}{
		{
			old: makefileOldObjectGenWithBoilerplateAndYear,
			new: makefileNewObjectGenWithBoilerplateAndYear,
		},
		{
			old: makefileOldObjectGenWithBoilerplate,
			new: makefileNewObjectGenWithBoilerplate,
		},
		{
			old: makefileOldObjectGenNoBoilerplate,
			new: makefileNewObjectGenNoBoilerplate,
		},
	}
	for _, replacement := range replacements {
		if err := util.ReplaceInFile(makefilePath, replacement.old, replacement.new); err == nil {
			return nil
		}
	}
	return fmt.Errorf("none of the known controller-gen object generator patterns matched")
}

Then the caller can stay simple:

if err := replaceObjectGenInMakefile(makefilePath); err != nil {
	log.Warn("unable to find standard controller-gen object generator line in Makefile. "+
		"Add applyconfiguration generation manually",
		"error", err)
	return
}

This avoids the nested if err != nil chain and removes the need for //nolint:lll.
Small note, it assumes ReplaceInFile returning an error means the pattern did not match in this context. If it can also fail because of read or write errors, it may be worth distinguishing those cases separately.

Can SSA also be cluster-scoped?

yes work with both.

I think the test exists only for namespaced, but no test for non-nemaspaced.

Just my thought

I was thinking would it be better than we return error from here on the reason why the updateMakefile failed and then callee here will handle the error and mention explicitly with log.Warn(err, "Failed to automatically update Makefile"). I feel this would make it much cleaner based on the logs on what exactly the issue is.

Note: We can still not fail the scaffolding process and let it continue

We should not return an error because that would means fail when the Makefile is customized and does not allow move forward if the end user change their Makefile.

If we be unable to update the Makefile we warn and then end users are aware of to update it manually

Also I was wondering, do we want to add a troubeshooting/manual steps if automatic update of Makefile failed. Currently we are just logging with warning messages.

We warn and end users can manually update their project.
It is too simple, has not really a reason for troubeshooting.
What we can do is provide a better warn message that can be more helpful.

Done 🚀