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

Escape special characters in jsonpath field names. #33901

Merged
merged 3 commits into from Oct 18, 2016

Conversation

jmcarp
Copy link
Contributor

@jmcarp jmcarp commented Oct 3, 2016

There may be a better way to do this, but this seemed like the simplest possible version.

Example: {.items[*].metadata.labels.kubernetes\.io/hostname}

[Resolves #31984]


This change is Reviewable

Example: `{.items[*].metadata.labels.kubernetes\.io/hostname}`

[Resolves kubernetes#31984]
@k8s-ci-robot
Copy link
Contributor

Can a kubernetes member verify that this patch is reasonable to test? If so, please reply with "@k8s-bot ok to test" on its own line.

Regular contributors should join the org to skip this step.

@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Oct 3, 2016
for {
r = p.next()
if isTerminator(r) {
switch r := p.next(); {
Copy link
Contributor

Choose a reason for hiding this comment

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

rather than do the labelled breaking, I'd introduce a new helper function:

func advance(p *Parser) bool {
  switch r := p.next(); {
     ...
  }
}

...
for advance(p) {}
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, made the change.

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 4, 2016
@brendandburns
Copy link
Contributor

/lgtm

@k8s-bot ok to test.

@brendandburns brendandburns added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Oct 4, 2016
@jmcarp
Copy link
Contributor Author

jmcarp commented Oct 5, 2016

@brendandburns: I see that the submit queue status check is still pending, but I don't see this PR in the queue at http://submit-queue.k8s.io/#/queue?prDisplay=33901&historyDisplay=33901. Do I need to do anything to get tests to run? Thanks!

@brendandburns brendandburns added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 6, 2016
@brendandburns
Copy link
Contributor

@k8s-bot ok to test

@brendandburns
Copy link
Contributor

@jmcarp sorry, I guess the bot doesn't like it when it's comment isn't the first thing on a line...

tests running now.

@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit 6787b67. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@brendandburns
Copy link
Contributor

@k8s-bot verification test this Issue: IGNORE

There are no failures that I can see...

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 14, 2016
@jmcarp
Copy link
Contributor Author

jmcarp commented Oct 14, 2016

I took another look at the build output, and it looks like I just had to run gofmt. @brendandburns: if you can get the k8s bot to re-run tests, hopefully they'll pass this time!

@brendandburns brendandburns added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 17, 2016
@brendandburns
Copy link
Contributor

Tests are green, LGTM!

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@fejta
Copy link
Contributor

fejta commented Oct 18, 2016

@k8s-bot verify test this

@k8s-github-robot
Copy link

Automatic merge from submit-queue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants