Skip to content

Regex Bugfix#999

Open
GuyARoss wants to merge 1 commit into
rancher:mainfrom
GuyARoss:main
Open

Regex Bugfix#999
GuyARoss wants to merge 1 commit into
rancher:mainfrom
GuyARoss:main

Conversation

@GuyARoss

@GuyARoss GuyARoss commented Jan 21, 2026

Copy link
Copy Markdown

We encountered an issue when utilizing this package as an informer sync mechanism where snake_case fields were not correctly being cached.

I traced the issue to this regex, modified it to accept snake_case, which seemed to fix the issue.

@GuyARoss GuyARoss requested a review from a team as a code owner January 21, 2026 20:41

@crobby crobby left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for your PR. It looks like it's breaking our current set of tests, which would need to be fixed.
Additionally, you would need to include new tests that cover the cases for which the regex is being modified.

@crobby crobby requested a review from ericpromislow January 22, 2026 11:26
@ericpromislow

ericpromislow commented Jan 23, 2026

Copy link
Copy Markdown
Contributor

The failed test is sometimes a flake, but this PR iguarantees failure. The failing test relies on a setting for a field called "_type". The passing test maps "_type" to "type" and succeeds in finding values in obj.type but not obj._type.

There's some object transformation going on as well that maps fields called "type" to "_type" so we can bypass metaprogramming that does other things on an object's type (sorry).

The test passes if you pull the _s out of the modified regex:

subfieldRegex = regexp.MustCompile(`([a-zA-Z0-9]+)|(\[[-a-zA-Z0-9./]+])|(\[[0-9]+])`)

But this doesn't help you at all ... you'll need to do some more work, possibly looking at the transforms
based at pkg/resources/virtual, to figure out what needs to be done.

@ericpromislow ericpromislow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unit tests fail. More work is needed to support field names containing underbars.

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.

3 participants