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

ensure include-meta-resources is supported consistently across kpt fn commands #2184

Open
droot opened this issue Jun 7, 2021 · 10 comments
Open
Labels
area/hydrate enhancement New feature or request size/S 1 day triaged Issue has been triaged by adding an `area/` label

Comments

@droot
Copy link
Contributor

droot commented Jun 7, 2021

No description provided.

@droot droot added enhancement New feature or request area/hydrate labels Jun 7, 2021
@droot droot added this to the v1.0 m3 milestone Jun 7, 2021
@droot droot added the size/S 1 day label Jun 7, 2021
@mikebz mikebz modified the milestones: v1.0 m3, v1.0 m4 Jun 9, 2021
@mikebz mikebz added the triaged Issue has been triaged by adding an `area/` label label Jun 10, 2021
@droot
Copy link
Contributor Author

droot commented Jun 10, 2021

Mostly likely, no work is required. I would like @phanimarupaka to audit the fn commands to see if everything is in place and consistent.

@droot droot modified the milestones: v1.0 m4, v1.0 m3 Jun 10, 2021
@phanimarupaka
Copy link
Contributor

phanimarupaka commented Jun 12, 2021

@droot kpt fn render should also support include-meta-resources flag consistent with kpt fn eval. With this flag, eval is more powerful and the functionality provided imperatively should also be provided declaratively.

Use case:
Consider an example where you want to set namespace and and also have the namespace value as substring of another field.

# mypkg/deployment.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: foo-deployment # kpt-set: ${ns}-deployment
  namespace: foo
# mypkg/Kptfile
apiVersion: kpt.dev/v1alpha2
kind: Kptfile
metadata:
  name: example
pipeline:
  mutators:
    - image: gcr.io/kpt-fn/apply-setters:v0.1
      configPath: setters-config.yaml
    - image: gcr.io/kpt-fn/set-namespace:v0.1
      configPath: namespace-config.yaml
# mypkg/setters-config.yaml
apiVersion: v1
kind: ConfigMap
metadata:
  name: setters-config
data:
  ns: myspace
# mypkg/namespace-config.yaml
apiVersion: v1
kind: ConfigMap
metadata:
  name: ns-config
data:
  namespace: foo # kpt-set: ${ns}

So using eval users can just update setter value to myspace ONCE and it can update the other function configs as well

kpt fn eval -i gcr.io/kpt-fn/apply-setters:v0.1 --fn-config setters-config.yaml --include-meta-resources
kpt fn eval -i gcr.io/kpt-fn/set-namespace:v0.1 --fn-config namespace-config.yaml
# mypkg/namespace-config.yaml
apiVersion: v1
kind: ConfigMap
metadata:
  name: ns-config
data:
  namespace: myspace # kpt-set: ${ns}
# mypkg/deployment.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: myspace-deployment # kpt-set: ${ns}-deployment
  namespace: myspace

This functionality is not offered by kpt fn render currently. Users must update both setters-config.yaml and namespace-config.yaml with value myspace and render the package to get the equivalent functionality of eval.

So we should support, --include-meta-resources feature for kpt fn render. But not sure if it should be offered as a flag for entire render command. We want it specified per function. So add a field to Function definition in Kptfile?

@phanimarupaka phanimarupaka modified the milestones: v1.0 m3, v1.0 m4 Jun 15, 2021
@mikebz mikebz removed this from the v1.0 m4 milestone Jul 14, 2021
@phanimarupaka
Copy link
Contributor

@droot @etefera Related to function selectors.

@bgrant0607
Copy link
Contributor

kpt fn render still doesn't have --include-meta-resources? Is there another way to get that behavior?
#1280 (comment)

@bgrant0607
Copy link
Contributor

cc @justinsb

@phanimarupaka
Copy link
Contributor

@bgrant0607 Currently no. We are planning to include it as part of selectors. Can you please explain your use-case explaining why you need it ?

@bgrant0607
Copy link
Contributor

What I'm doing now is copying Kptfile info (current the name) into a ConfigMap so that it can be read during kpt fn render.

@bgrant0607
Copy link
Contributor

bgrant0607 commented Oct 22, 2021

Example code that look for the package name copied into a ConfigMap and then uses it to change the namespace of the resources in the package. I'm calling this a "variant constructor" pattern.

func SetNamespace(ctx *krmfunction.Context, in []*yy.KubeObject, config map[string]string) ([]*yy.KubeObject, error) {
	setNamespace := ""
	for _, obj := range in {
		apiVersion := obj.GetAPIVersion()
		kind := obj.GetKind()
		oname := obj.GetName()
		if apiVersion == "v1" && kind == "ConfigMap" && oname == "kptfile.kpt.dev" {
			data, err := obj.GetOrCreateMap("data")
			if err != nil {
				return nil, err
			}
			setNamespace, _ = data.GetString("name")
			break
		}
	}
	if setNamespace == "" {
		return in, nil
	}
	for _, obj := range in {
		apiVersion := obj.GetAPIVersion()
		kind := obj.GetKind()
		if apiVersion == "v1" && kind == "Namespace" {
			if err := obj.SetName(setNamespace); err != nil {
				return nil, err
			}
			continue
		}
		if !obj.HasNamespace() {
			continue
		}
		if err := obj.SetNamespace(setNamespace); err != nil {
			return nil, err
		}
	}
	return in, nil
}

@bgrant0607
Copy link
Contributor

I also think we may need to collect information from the environment and record it as local pseudo-resources, which kpt right now treats as meta-resources, to use as well known inputs in the function pipeline. For instance, the values we were treating as autosetters previously could be recorded in a context object.

@droot
Copy link
Contributor Author

droot commented Feb 16, 2022

ref: #2781

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 size/S 1 day triaged Issue has been triaged by adding an `area/` label
Projects
None yet
Development

No branches or pull requests

4 participants