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

Use existing values to support partial updates on alerts and actions #49827

Closed
mikecote opened this issue Oct 31, 2019 · 5 comments
Closed

Use existing values to support partial updates on alerts and actions #49827

mikecote opened this issue Oct 31, 2019 · 5 comments

Comments

@mikecote
Copy link
Contributor

Right now you need all the attributes part of the AAD on update.

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-stack-services (Team:Stack Services)

@pmuellr
Copy link
Member

pmuellr commented Nov 1, 2019

So you're suggesting the update endpoints of alerts/actions should support a 'merge' (update, not replace) option?

I think that makes sense, but our current 'overwrite' story (you need to provide all the attributes, and then weirdly, we do an update, not overwrite, on the SO) is very secure, AAD wise. This seems like it may change that story a bit.

@mikecote
Copy link
Contributor Author

mikecote commented Nov 1, 2019

@pmuellr Hmm, good point, maybe we shouldn't do this because of AAD. It would probably go against why we're using AAD in the first place. We will have to live with some weirdness like this (#49320 (comment)) by keeping what we have now, which makes sense I guess.

@pmuellr
Copy link
Member

pmuellr commented Nov 1, 2019

Ya, I have this sneaking suspicion it may break AAD, or the security it provides anyway.

Since we currently only support 'overwrite' (but it's really an SO update), and will always need to, in case someone wants to "delete" an attribute by setting it to null, I think we'll need a separate mode for 'update' (option provide on update request), and so seems like something we can defer till we see a requirement. Eg, I can see how our current API could be extended in the future to provide this capability.

Do we have a requirement for this from someone?

@mikecote
Copy link
Contributor Author

mikecote commented Nov 1, 2019

None at this time, it was more a quick thought I had that seemed redundant but as soon as "breaking AAD" was brought up, made this issue not recommended. I will close for now and we can revisit in the future if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants