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

Saved object update w/ upsert doesn't work for multi-namespace objects that don't exist #114918

Closed
jportner opened this issue Oct 13, 2021 · 2 comments · Fixed by #114929
Closed
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Security/Sharing Saved Objects Platform Security - Sharing Saved Objects feature Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!

Comments

@jportner
Copy link
Contributor

Kibana version: 7.14+

Describe the bug:

I uncovered this while troubleshooting some bugs in #114620. That PR changes the visualization saved object to use namespaceType: 'multiple-isolated' (to become share-capable). In the process, an API integration test started failing:

it('handles upsert', async () => {
await supertest
.put(`/api/saved_objects/visualization/upserted-viz`)
.send({
attributes: {
title: 'foo',
},
upsert: {
title: 'upserted title',
description: 'upserted description',
},
})
.expect(200);

This first assertion failed with a 404 error. Digging into it, I realized that it's because the upsert handling in the SavedObjectsRepository (SOR) is flawed:

let preflightResult: SavedObjectsRawDoc | undefined;
if (this._registry.isMultiNamespace(type)) {
preflightResult = await this.preflightCheckIncludesNamespace(type, id, namespace);
}
const time = getCurrentTime();
let rawUpsert: SavedObjectsRawDoc | undefined;
if (upsert) {
let savedObjectNamespace: string | undefined;
let savedObjectNamespaces: string[] | undefined;
if (this._registry.isSingleNamespace(type) && namespace) {
savedObjectNamespace = namespace;
} else if (this._registry.isMultiNamespace(type)) {
savedObjectNamespaces = await this.preflightGetNamespaces(type, id, namespace);
}
const migrated = this._migrator.migrateDocument({
id,
type,
...(savedObjectNamespace && { namespace: savedObjectNamespace }),
...(savedObjectNamespaces && { namespaces: savedObjectNamespaces }),
attributes: {
...upsert,
},
updated_at: time,
});
rawUpsert = this._serializer.savedObjectToRaw(migrated as SavedObjectSanitizedDoc);
}

Usage of the preflightCheckIncludesNamespace method was added before the upsert functionality. That will throw a 404 error if the object doesn't exist in the current space. So if you try to use upsert for an object that doesn't exist, it will always fail with a 404 error.

Steps to reproduce:

Use #114620 and observe the failing integration test.

Expected behavior:

If upsert is NOT used:

  1. Updating an object when it exists in the current space should succeed
  2. Updating an object when it exists, but not in the current space, should fail with a 404
  3. Updating an object when it doesn't exist should fail with a 404

If upsert IS used:

  1. Updating an object when it exists in the current space should succeed
  2. Updating an object when it exists, but not in the current space, should fail with a 404
  3. Updating an object when it doesn't exist should succeed
@jportner jportner added bug Fixes for quality problems that affect the customer experience Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! labels Oct 13, 2021
@jportner jportner self-assigned this Oct 13, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Security/Sharing Saved Objects Platform Security - Sharing Saved Objects feature Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants