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

Add ability to specify pipeline function config with selector #3304

Open
andsens opened this issue Jun 10, 2022 · 5 comments
Open

Add ability to specify pipeline function config with selector #3304

andsens opened this issue Jun 10, 2022 · 5 comments
Labels
area/hydrate enhancement New feature or request good first issue Good for newcomers p2 triaged Issue has been triaged by adding an `area/` label

Comments

@andsens
Copy link

andsens commented Jun 10, 2022

Given this config

# Kptfile
apiVersion: kpt.dev/v1
kind: Kptfile
metadata:
  name: example
pipeline:
  mutators:
  - image: gcr.io/kpt-fn/apply-replacements:v0.1.1
    configPath: replace.yaml
# replace.yaml
apiVersion: fn.kpt.dev/v1alpha1
kind: ApplyReplacements
metadata:
  name: replace
  annotations:
    config.kubernetes.io/local-config: "true"
replacements:
- source:
    kind: ConfigMap
    name: some-config
    fieldPath: data.test
  targets:
  - select:
      kind: Deployment
      name: deployment.example
    fieldPaths:
    - metadata.name

I would love to see the ability to reference the function config like this instead:

[...]
  mutators:
  - image: gcr.io/kpt-fn/apply-replacements:v0.1.1
    configSelector:
      # No "kind" selector necessary since it's always "ApplyReplacements"
      name: replace

This change carries with it quite a few advantages:

  • You are no longer limited to specifying one config per file. Configs that belong together semantically can reside in the same file.
  • Preceeding pipeline functions can actually generate configs for subsequent functions since we don't reference a specific file that must exist from the get-go.
  • It seems to be more in line with how the rest of kpt works, where filepaths are just metadata during processing.
  • If preprocessing ever becomes a thing, a child package could change how a parent package behaves, allowing for more concise setups

The disadvantage I see is mostly the introduction of footguns:

  • How a function behaves can be quite convoluted, if e.g. its config is changed multiple times before being applied.
  • You no longer have the WYSIWYG guarantee for function configs: When debugging you'll always have to keep in mind that a bug might be caused by an inadvertent config change (though the configs are all their own kind, so it might not be that likely to happen unless using completely unguarded mutations)
  • Finding a function config is not as direct. If you're unfamiliar with the package you'll spend time searching through it to find the referenced config.
@andsens andsens added the enhancement New feature or request label Jun 10, 2022
@bgrant0607
Copy link
Contributor

bgrant0607 commented Jun 30, 2022

cc @natasha41575 @droot @yuwenma

@bgrant0607
Copy link
Contributor

For cases where there is a specific KRM type that maps to a specific function, as with ApplyReplacements, my preferred solution is a function catalog mechanism, so that it doesn't need to be specified in the Kptfile at all.
https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/2906-kustomize-function-catalog

We just added a temporary hack to add the apply-replacements function to the Kptfile automatically in the UI, because I forget to do that half the time when running demos.
GoogleContainerTools/kpt-backstage-plugins#56

@bgrant0607
Copy link
Contributor

Filed #3339 for the catalog-based mapping mechanism.

Are there other use cases where a selector would be desirable?

In general, I do think we need more flexibility with respect to where inputs come from. Other examples:
#3155 (comment)
#3280 (comment)

@bgrant0607
Copy link
Contributor

A good point was raised in slack:
https://kubernetes.slack.com/archives/C0155NSPJSZ/p1656746811019709

It seems more appropriate to use KRM-based references and selectors than file paths.

We need to think about how this would interplay with other mechanisms, such as replay-based updates: #3329. And bulk package creation: #3347.

@droot droot added the good first issue Good for newcomers label Jan 26, 2023
@droot
Copy link
Contributor

droot commented Jan 26, 2023

This is a good candidate someone to contribute to kpt, will be more than happy to help someone guide how to get this done.

@droot droot added triaged Issue has been triaged by adding an `area/` label p2 labels Jan 26, 2023
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 good first issue Good for newcomers p2 triaged Issue has been triaged by adding an `area/` label
Projects
None yet
Development

No branches or pull requests

4 participants