Creating a substitution doesn't check for existing substitution with the same name

[mortent]

We check whether there are any existing setters with the same name, but we don't check for any existing substitutions. This can cause users to inadvertently replace existing setters.

[phanimarupaka]

@mortent Doesn't this mean user is trying to modify existing substitution? We should not throw error in such scenario and update the existing substitution which is the behavior currently.

[mortent]

But the current behavior seems broken. This will update the definition of the substitution in the Kptfile and add the openapi ref to the new fields, but it doesn't clean up the existing openapi references in the manifests or check if this substitution is references by other substitutions.

An example is https://github.com/mortent/solutions-modern-cicd-anthos/tree/KptFunction/krm/kpt/cicd_cluster which has a substitution cluster-name that is used by several other substitutions. If I do something like

kpt cfg create-subst . cluster-name --field-value anthos-platform --pattern \${platform}-platform

Things end up in pretty weird state.

I'm not sure what is the desired behavior here, but it seems clear that we at least would need to remove the existing openapi references to the substitution. What is the use-case for updating a substitution vs just requiring users to remove it and re-create it?

[phanimarupaka]

There is no delete substitution command yet. But yeah, I agree that we need to atleast have some flag like --update-if-exists to modify it. The default behavior for create-subst or create-setter should be to throw an error. So in this case, we should first implement delete-subst command before we implement throwing an error. WDYT?