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

Rename namespaces to initialNamespaces for create operations #79682

Merged
merged 2 commits into from
Oct 6, 2020

Conversation

jportner
Copy link
Contributor

@jportner jportner commented Oct 6, 2020

In #76132 we added the ability to create multi-namespace saved objects in an initial set of consumer-defined namespaces (via "create" and "bulkCreate" APIs). However, retrieving objects via "get", "bulkGet", or "find" will also return each object's current namespace(s) in a namespaces field. If a consumer is attempting to create a new object based on that result, they will unintentionally attempt to set the new object's initial namespaces.

To avoid this behavior, we'll rename these fields to initialNamespaces for create operations so they cannot be accidentally conflated with the results of read operations.

@@ -44,16 +44,16 @@ export const registerCreateRoute = (router: IRouter) => {
})
)
),
namespaces: schema.maybe(schema.arrayOf(schema.string(), { minSize: 1 })),
initialNamespaces: schema.maybe(schema.arrayOf(schema.string(), { minSize: 1 })),
Copy link
Contributor

Choose a reason for hiding this comment

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

If beats is sending namespaces in it's payload, won't it fail validation here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would fail using the external create API route, but not using the legacy import API (which does not validate object fields)

Copy link
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

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

LGTM on green CI

@jportner
Copy link
Contributor Author

jportner commented Oct 6, 2020

Test failure looks unrelated, probably CI madness due to all the merges today, I'm going to merge upstream / retest to see if that fixes it

@jportner
Copy link
Contributor Author

jportner commented Oct 6, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants