set-project-id relies on apply-setter to edit project

[yuwenma]

As the name shows, set-project-id should be safely used as a declarative function in Kptfile. However, it is only used as an imperative function that changes Kptfile only:
Running kpt fn eval --image gcr.io/kpt-fn/set-project-id adds a new apply-setter function to Kptfile. See README.

The set-project-id cannot be used in Kptfile directly. errors

failed to set project-id setter: ... please include --include-meta-resources flag if a Kptfile is present

include-meta-resources is not supported in kpt fn render. Even if supported, it won't edit the actual resource files but only change Kptfile file.

Under the hood, set-project-id relies on apply-setter to do the real work. This is an anti-pattern of relying on set-project-id and other specific functions to build a setter free configurations.

[morgante]

Was this a regression? It seems like he original purpose of set-project-id has significantly diverged from what it was meant to be in the first place. go/set-project-id

[yuwenma]

@morgante Correct. Current set-project-id is a regression and also introduces a bug to KCC blueprints:
Though set-project-id adds annotation cnrm.cloud.google.com/project-id, it doesn't give it the setter comment which means the annotation's project-id will be out of sync with other project-id once apply-setter runs.

I see the rationale behind set-project-id from go/set-project-id. I propose we remove set-project-id for now and I will come up with a solution (hopefully within this week).

[morgante]

I am not supportive of removing set-project-id. It sounds like it was poorly implemented in the first place, but we should fix it instead of removing it.

I'm still not sure I follow the issue. If you don't use setters at all, does set-project-id not work?

[yuwenma]

"remove set-project-id" --> Rename set-project-id to something imperative (like add-project-id-setter) since it can only be run as imperative function. And also fix the issues I mentioned.

As a workaround before a setter free solution, I'm fine with the function to rely on apply-setter and modify the Kptfile file. But it should not be treated as curated and should not use misleading names.

[morgante]

"remove set-project-id" --> Rename set-project-id to something imperative (like add-project-id-setter) since it can only be run as imperative function. And also fix the issues I mentioned.

It was never intended to be "imperative-only." It sounds like the original implementation was done incorrectly, as go/set-project-id was meant to be usable in both imperative and declarative workflows.

I have no interest in a separate "imperative-only" function. Please focus on updating the existing function to match the spec I provided.