Skip to content
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

Support a tool-agnostic concise setter anchor format #2369

Closed
bgrant0607 opened this issue Jul 7, 2021 · 8 comments
Closed

Support a tool-agnostic concise setter anchor format #2369

bgrant0607 opened this issue Jul 7, 2021 · 8 comments
Labels
area/hydrate enhancement New feature or request triaged Issue has been triaged by adding an `area/` label

Comments

@bgrant0607
Copy link
Contributor

This has been mentioned in a couple places:
#985 (comment)
kubernetes-sigs/kustomize#3980 (comment)

If/when setters are added as a transformer in kustomize (kubernetes-sigs/kustomize#3953 (comment)), it would be helpful for users if the setter anchors in resources were interoperable. That would be facilitated if a string without "kpt" in it (as in "kpt-set") were (also) supported in both tools.

Some possible usage scenarios, in addition to some more general scenarios in #1447:

  • Use kustomize to perform on-the-fly out-of-place hydration on a kpt package base
  • Use kpt fn eval to update in-line setter values in kustomize base resources
@bgrant0607 bgrant0607 added the enhancement New feature or request label Jul 7, 2021
@bgrant0607
Copy link
Contributor Author

Actually, it seems like it shouldn't be hard to make the anchor string a configurable function parameter, with kpt-set as the default.

@marshall007
Copy link

Is standardizing the anchor format enough? I think we might also need to standardize where setter values are sourced from.

If you are manipulating a "package" using generic tooling, such tooling may not know how the setter values will ultimately be applied. If your generic tooling updates setter values in-place and downstream tooling like a kustomize transformer updates out-of-place based on gnostic configuration, you can provide no guarantee that your in-place edits will "stick".

FWIW, I think the pre-v1 solution for how setters were defined and manipulated (i.e in the Kptfile) was a more robust (albeit verbose) approach to accommodating such scenarios. All that was really lacking from our perspective was standardization between Krmfile and Kptfile. We also found the automated doc generation functionality very useful and it will be missed.

@bgrant0607
Copy link
Contributor Author

@marshall007 Yes, for the use cases I mentioned making the anchor format configurable is enough.

The original setter design didn't fit with either the kpt or kustomize function/transformer models. They worked differently from everything else and created challenges when used with the tools' other features.

Additionally, setters are not intended to provide interfaces to packages / configuration. They facilitate ad hoc transformations for cases where dedicated functions or transformers haven't been written yet. The transformation models supported by both tools are intended to mitigate overuse of parameters (https://github.com/kubernetes/community/blob/master/contributors/design-proposals/architecture/declarative-application-management.md#parameterization-pitfalls) in order to simplify configuration.

Function parameter validation and descriptions are needed more generally, not just for setters, as mentioned here:
#2041 (comment)

@marshall007
Copy link

Additionally, setters are not intended to provide interfaces to packages / configuration. They facilitate ad hoc transformations for cases where dedicated functions or transformers haven't been written yet. [...]

@bgrant0607 thanks, your explanation makes sense and I can get behind that. I think the default anchor format should be defined as a constant in cli-utils that everyone can share. It's fine if the user wants to configure something other than that, but why not go ahead and make the tooling interoperable out of the box?

The original setter design didn't fit with either the kpt or kustomize function/transformer models. They worked differently from everything else and created challenges when used with the tools' other features.

I am still concerned that this problem is only partially solved with the new setter design and creates new tooling-specific incompatibilities.

For kustomize, you were suggesting that setters be implemented as an on-the-fly transformer. This made sense in the context of applying setters to remote packages, but how would we reconcile this with kustomize's own support for declarative function execution (via kustomize fn run)?

It seems like we are back to having no tooling-agnostic way to declare and execute in-place edits. Now it's just a more general problem of encapsulating an entire pipeline vs being unique to setters.

The new declarative function execution, kpt fn render, and pipeline definitions in the Kptfile make a lot more sense to me than kpt fun run <dir> ever did, but I don't understand why this was not implemented in kustomize. If it were then we could solve the problem related to remote packages simply by executing kustomize fn render after a all packages are downloaded, no transformer necessary.

@mikebz
Copy link
Contributor

mikebz commented Jul 9, 2021

@droot heads up

@frankfarzan
Copy link
Contributor

frankfarzan commented Jul 9, 2021

The current approach to setters is having it be a KRM function which can be used by any orchestrator (kpt, kustomize, etc.) that follows the KRM functions spec. The old way of setters baked into the orchestrator is deprecated and will be cleaned up. See kubernetes-sigs/kustomize#4045

The choice of comment format (e.g. kpt-set) doesn't affect this interoperability. It's chosen as a namespacing mechanism to avoid conflict between different functions developed by different vendors. There's also kpt-merge and kpt-include, etc. used in other functionality provided by the either kpt CLI or functions in gcr.io/kpt-fn/. Third-party custom functions can choose their own identifier. For new third-party functions, we may want to consider recommending a domain naming convention to avoid any accidental conflicts (similar to annotations). Keeping in mind we don't want these to be overly verbose so they can still be human authored.

@frankfarzan
Copy link
Contributor

/cc @phanimarupaka

@bgrant0607
Copy link
Contributor Author

We don't intend to invest more in setters. See #3131.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hydrate enhancement New feature or request triaged Issue has been triaged by adding an `area/` label
Projects
None yet
Development

No branches or pull requests

4 participants