-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Account for null
metadata in update API key test
#89195
Conversation
null
metadata in update API key test
"for api key doc " + actualRawApiKeyDoc, | ||
// Internally, metadata may be stored as `null`. However, it is always exposed as an empty map through the API. We define | ||
// `expectedMetadata` as the expected value according to the API, so we need to account for this discrepancy here | ||
actualMetadata == null ? anEmptyMap() : actualMetadata, |
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.
I'm addressing the issue here, instead of going the other direction and accounting for null
values on the GET API response. I think it is more intuitive to treat the API response as the expected result. We also run into an issue with null pointers if we try to pass a null value into expectAttributesForApiKey
via Map.of(METADATA, null)
.
I would be up for only asserting on the API response, and removing expectMetadataForApiKey
altogether.
Pinging @elastic/es-security (Team:Security) |
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
Thanks for picking this up.
When the metadata field on the raw API key doc is
null
, the GET APIautomatically translates it to an empty map. This PR fixes a failing
test by accounting for this difference in a test assertion.
Closes #89193