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
Fix failing unit test for parseImportID #1426
Conversation
@@ -50,15 +50,15 @@ func TestParseImportID(t *testing.T) { | |||
t.Fatal(actualErr.Error()) | |||
} | |||
if expected.GVK != actualGvk { | |||
t.Log("GVK (actual / wanted):", actualGvk, expected.GVK) | |||
t.Logf("GVK did not match. actual: %q expected: %q.", actualGvk, expected.GVK) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took me a minute to figure out which case was actually failing so I made this a bit more readable.
I actually took the opposite approach in my PR because I would think cluster scoped resources (like ClusterRole, or MutatingWebhookConfiguration) wouldn't need to carry around a namespace in their TF ID. If I'm wrong about that, I'm definitely open to modifying my PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Nice improvement!
Thanks for commenting @keithmattix – I think it has to be this way around, otherwise people will always have to specify a |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Description
This unit test was failing because parseImportID will set namespace to default if it's not present. We would have to check if the kind was a namespaced resource to figure out if we should omit it or not. AFAIK namespace should just get ignored for non-namespaced resources so this should be fine for now.
Acceptance tests
Output from acceptance testing:
Release Note
Release note for CHANGELOG:
References
Community Note