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

feat: support .metadata.generateName (#526) #529

Merged
merged 1 commit into from
Mar 18, 2021

Conversation

wojciechka
Copy link
Contributor

Adds support for tk show and tk export to handle K8s objects that use .metadata.generateName instead of .metadata.name.

Changes the default format (being backwards compatible) to also support outputting such files with tk export.

@wojciechka wojciechka changed the title chore: support .metadata.generateName (#526) feat: support .metadata.generateName (#526) Mar 5, 2021
Copy link
Member

@Duologic Duologic left a comment

Choose a reason for hiding this comment

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

LGTM, let's wait for a bit of input.

@@ -62,7 +62,7 @@ func (m Manifest) Verify() error {
if !o.Get("metadata").IsMSI() {
fields["metadata"] = ErrInvalidMap
}
if !o.Get("metadata.name").IsStr() {
if !o.Get("metadata.name").IsStr() && !o.Get("metadata.generateName").IsStr() {
Copy link
Member

Choose a reason for hiding this comment

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

I don't know this code very well, not sure if we need to improve the error message too. @sh0rez

@Duologic Duologic merged commit d548e54 into grafana:master Mar 18, 2021
@wojciechka
Copy link
Contributor Author

@Duologic Thanks for merging this. I was not sure how long I should wait and this was not urgent for me, so thank you for handling it.

@Duologic
Copy link
Member

Yeah, I'm still not sure about the error message, but I think progress over perfection is fine here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants