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

option for origin data as annotation #4065

Merged

Conversation

natasha41575
Copy link
Contributor

@natasha41575 natasha41575 commented Jul 13, 2021

Addresses #3979

This PR adds a top-level field to the kustomization:

buildMetadata: [originAnnotations]

This will add the following annotation to local files:

config.kubernetes.io/origin: |
  path: ../base/deployment.yaml

where path is the path to the file where the resource originated from, rooted at the directory upon which kustomize build is invoked.

For resources from remote bases, e.g. with the following kustomization.yaml:

resources:
- github.com/kubernetes-sigs/kustomize/examples/multibases/dev/?ref=v1.0.6
buildMetadata: [originAnnotations]

this will add the following annotation

config.kubernetes.io/origin: |
  path: examples/multibases/base/pod.yaml
  repo: https://github.com/kubernetes-sigs/kustomize
  ref: v1.0.6

where path is the path to the resource rooted at the root of the remote repository. ref will be omitted if the remote URL doesn't have one.

See tests at buildoptions_test.go and remoteload_test.go.

ALLOW_MODULE_SPAN

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 13, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: natasha41575

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 13, 2021
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 13, 2021
@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 16, 2021
@natasha41575 natasha41575 removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 22, 2021
@monopole monopole mentioned this pull request Jul 25, 2021
Copy link
Contributor

@monopole monopole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling this. It's complicated.

api/krusty/remoteload_test.go Show resolved Hide resolved
api/krusty/remoteload_test.go Outdated Show resolved Hide resolved
api/krusty/remoteload_test.go Show resolved Hide resolved
api/krusty/remoteload_test.go Outdated Show resolved Hide resolved
api/krusty/remoteload_test.go Outdated Show resolved Hide resolved
@@ -151,20 +153,20 @@ func (kt *KustTarget) addHashesToNames(
// holding customized resources and the data/rules used
// to do so. The name back references and vars are
// not yet fixed.
func (kt *KustTarget) AccumulateTarget() (
func (kt *KustTarget) AccumulateTarget(origin *resource.Origin) (
Copy link
Contributor

@monopole monopole Jul 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an important entry point, and its argument should be explained.

Does the method change it / take ownership of it (presumably yes, since a ptr is sent).
Is the outside owner expected to consult it?
What makes it change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a comment explanation

api/types/buildoptions.go Outdated Show resolved Hide resolved
api/types/kustomization.go Outdated Show resolved Hide resolved
api/internal/target/kusttarget.go Outdated Show resolved Hide resolved
}
if origin.Ref != "" {
anno = anno + "ref: " + origin.Ref + "\n"
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, this anno construction code should live in origin, with regression coverage (examples) in origin_test.go

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved and tests are added in origin_test.go

@natasha41575 natasha41575 force-pushed the originDataAsAnnotation branch 2 times, most recently from 9157d46 to 4769871 Compare August 6, 2021 00:21
@natasha41575
Copy link
Contributor Author

@KnVerey @monopole PTAL

@natasha41575
Copy link
Contributor Author

@yuwenma

Copy link
Contributor

@monopole monopole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

thanks!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 19, 2021
@monopole monopole merged commit 28707bf into kubernetes-sigs:master Aug 19, 2021
@ringerc
Copy link

ringerc commented Nov 22, 2021

This is brilliant, it's exactly what I've been looking for.

I don't see any way to specify the top-level origin without invoking kustomize directly on a remote base though. If I'm looking at a deployed resource with

metadata:
  annotations:
    config.kubernetes.io/origin: |
      path: serviceAccount.yaml

ideally I want to see something about where that serviceAccount.yaml came from.

I presume the best option here is to inject your own annotations with Kustomize commonAnnotations at the top level with an originating repo URI or the like. That should work well enough, though it'll be a bit fragile with layering. Ideally I'd want to be able to tell Kustomize the "origin" of the top level, so it records top-level origins fully qualified as well.

In any case, brilliant to see this, and thankyou.

@natasha41575
Copy link
Contributor Author

natasha41575 commented Nov 22, 2021

@ringerc I'm not sure I understand the use case. If you use a remote base, you should see the repo and ref in the origin annotation as well.

e.g.

kustomization.yaml

resources:
- github.com/kubernetes-sigs/kustomize/examples/multibases/dev/?ref=v1.0.6
buildMetadata: [originAnnotations]

will have the following annotation:

config.kubernetes.io/origin: |
  path: examples/multibases/base/pod.yaml
  repo: https://github.com/kubernetes-sigs/kustomize
  ref: v1.0.6

Is this a different scenario from the one you are describing?

EDIT: I think I understand what you mean actually. Yeah, I think your use case is best done by using commonAnnotations. This feature only supports telling you where the resources originated relative to where kustomize build is run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants