kpt fn exclude design proposal

[natasha41575]

Design proposal for kpt fn exclude.

Issue: #2930
Implementation PR: #2957

[droot]

@natasha41575 the proposal is looking good to me. We can also get a quick review from @bgrant0607 about it.

[bgrant0607]

The SDK should make filtering out these local config objects easy.

[yuwenma]

Yes, set-namespace excludes local-config resources inside its logic.

To have SDK filter out resources, we can check the type for Kptfile, but how can we tell which resources are function configs if not using a new annotation nor reusing local-config?

[yuwenma]

To be more specific, in a hydration pipeline if function1 and function2 have different function config funcitonConfig1 and functionConfig2 correspondingly, the resourceslist for function1 is.

kind: ResourceList
functionConfig: 
    kind: functionConfig1
    ...
items:
- kind: Kptfile
   ...
- kind: functionConfig1
   ...
- kind: functionConfig2
   ...

the resourceslist for function2 is.

kind: ResourceList
functionConfig: 
    kind: functionConfig2
    ...
items:
- kind: Kptfile
   ...
- kind: functionConfig1
   ...
- kind: functionConfig2
   ...

function1 does not know which input resource is function2's config so it cannot skip it, and vice versa.

[natasha41575]

In what cases will the user need to skip functionConfig? If they are our own horizontal functions like set-namespace, then setting the namespace of a functionConfig doesn't seem to really change or break any use cases. And if the user really wants to skip these functionConfigs, then they could use kind-based exclusion since every function has its own functionConfig kind, like SetNamespace. WDYT?

[yuwenma]

I misread Brians comments. I thought his suggestion is an alternative which the SDK filtering out FunctionConfigs and not having Kptfile GVK level syntax. Now I see these are in the Open Questions, not alternatives.

[bgrant0607]

I disagree this is easy to understand. It doesn't make sense to me. The right behavior entirely depends on what the function is doing. The logic really belongs in the function SDK.

[natasha41575]

I see your point, and totally agree that the logic belongs in the function SDK and should be controlled on a per-function level. What I was trying to say here is that the annotation is descriptive and clearly tells the user what it means. But I agree that it isn't clear or easy to understand why we would go this route, so I've taken out this line from the proposal.

[bgrant0607]

This would break all functions that use local config pseudo-resources as implicit inputs. I had to workaround this behavior when we built the porch prototype.

[yuwenma]

My bad. I proposed this to simplify the annotations users need to deal with.

[natasha41575]

Thanks for the review @bgrant0607! I updated the proposal based on your feedback. If you have any further feedback, let me know. I plan to merge this design proposal after your approval.

[yuwenma]

@bgrant0607 I think there's a misalignment, just want to call it out. @natasha41575 Correct me if I myself misunderstand it.

This proposal is motived from excluding Kptfile from resources, but it is trying to solve a bigger problem which is described in the "Prior art in kustomize" that "exclude certain resources from being modified ... is one of the most popular among kustomize users". So it is not limited to Kptfile, but skip any types of KRM resources.

Thus, my understanding is that different customers have different resource skipping list, and it won't be a function level feature, (thus the design focus is on what's the best syntax to skip those resources easily and reliably). When you suggest this filtering to be a function level feature, do you mean the function should provide a general solution on accepting a custom FieldSpec list, iterate and skip resources from this list? The general syntax for such a function config might be

apiVersion: kpt.dev.fn/v1alpha1
kind: SetXXX
exclude:
- kind: ConfigMap
- apiVersion: kpt.dev.fn/v1alpha1

[natasha41575]

@yuwenma My understanding of what it means for filtering to be a "function level" feature from my discussions with @droot is that the user should be able to control which resources are included for each function individually, rather than have kpt make the same decision for all functions in the pipeline. The design proposal currently is in alignment with that, because the user can specify selectors/exclude for each function separately.

[yuwenma]

@yuwenma My understanding of what it means for filtering to be a "function level" feature from my discussions with @droot is that the user should be able to control which resources are included for each function individually, rather than have kpt make the same decision for all functions in the pipeline. The design proposal currently is in alignment with that, because the user can specify selectors/exclude for each function separately.

Ah, saw the other comment.
I think we shall define two phases for this exclude design proposal.
Since the current solution is more of a workaround and eventually the filtering logic should be in the SDK.

• Phase 1. As the current proposal says, filtering the resources before passing into the function.
• Phase 2.
  - kpt shall annotate Kptfile and inline ConfigMap with an internal annotation, propose internal.config.kubernetes.io/kpt-meta, the SDK should provide functions to filter resources by annotation (and this should be called inside a function)
  - FunctionConfig defines a general interface to filter resources by GVK. SDK provides the actual filtering logic (maybe a default behavior under the SDK resourceList Process so it's applied to all functions automatically). It would be like

apiVersion: config.kubernetes.io/v1
kind: Resourcelist
items:
- apiVersion: v1
  kind: ConfigMap
  metadata:
       name: current-function-config
       annotations:
         internal.config.kubernetes.io/kpt-meta: true # marked as meta resource, exclude by default
- apiVersion: v1
  kind: ConfigMap
  metadata:
       name: other-function-config
  annotations:
       internal.config.kubernetes.io/kpt-meta: true # marked as meta resource, exclude by default
- apiVersion: kpt.dev/v1
  kind: Kptfile   # Kptfile, exclude by default
  ...
functionConfig:
  apiVersion: v1
  kind: ConfigMap
  metadata:
      name: current-function-config
      annotations:
        internal.config.kubernetes.io/exclude-meta: true
        internal.config.kubernetes.io/exclude: |     # function level exclude list.
           - kind:
           - kind:

[bgrant0607]

I'm fine with the exclude feature as described. It seems useful for horizontal functions like set-labels, where the function itself wouldn't necessarily know which resources should be targeted.

For the problem of changing behavior with respect to meta-resources and local resources, I see it as a temporary workaround.

Functions need to ignore resources and resource types they don't care about or would operate incorrectly on. The pipeline should always pass through everything by default.

The function SDKs need to make it straightforward to implement such filtering.

If some resources or resource types need to be made easier to identify, I'd be fine with introducing new intra-pipeline annotations to identify them.

ConfigMaps that are generated to be used as functionConfig from inline configMaps in the Kptfile should be automatically annotated as local-config objects. ConfigMaps and pseudo-resources in their own files would need to be annotated by their creators if they didn't want them to be instantiated in the cluster.

[natasha41575]

I'm fine with the exclude feature as described. It seems useful for horizontal functions like set-labels, where the function itself wouldn't necessarily know which resources should be targeted.

Sounds like we are all on board with the design as described, so I will go ahead and merge this PR along with the implementation PR.

The function SDKs need to make it straightforward to implement such filtering.

SGTM, and sounds like this would be something we discuss in Yuwen's phase 2 in her comment. @yuwenma should we file a separate tracking issue for that work?

[yuwenma]

I'm fine with the exclude feature as described. It seems useful for horizontal functions like set-labels, where the function itself wouldn't necessarily know which resources should be targeted.

Sounds like we are all on board with the design as described, so I will go ahead and merge this PR along with the implementation PR.

The function SDKs need to make it straightforward to implement such filtering.

SGTM, and sounds like this would be something we discuss in Yuwen's phase 2 in her comment. @yuwenma should we file a separate tracking issue for that work?

Filed #3016