[GODRIVER-1415] Add build flag to run specific commands without implicit sessions Created: 20/Nov/19  Updated: 27/Oct/23  Resolved: 22/Nov/19

Status: Closed
Project: Go Driver
Component/s: Administrative Commands
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Major - P3
Reporter: Divjot Arora (Inactive) Assignee: Unassigned
Resolution: Works as Designed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
Related

 Description   

This ticket is in response to HELP-12163. To allow the monitoring agent to run listCommands, _isSelf, and buildInfo without authenticating against affected 3.6 servers, we should add a build flag like 36nosessions. If specified during compilation, the driver will not add an implicit session for those commands if they are run through RunCommand. The driver will honor explicit sessions for those commands even if the build flag is specified.

 

CC thomas.delacour jeff.yemin jonathan.balsano louisa.berger I want to make sure everyone agrees with this solution before we implement anything in the driver.



 Comments   
Comment by Divjot Arora (Inactive) [ 22/Nov/19 ]

john.morales Thanks for the quick follow-up. Closing this issue as "Works as Designed".

Comment by John Morales [ 21/Nov/19 ]

Let me discuss this a little more within Cloud first. Will try and follow up before Monday afternoon.

Comment by Bernie Hackett [ 21/Nov/19 ]

After talking to Jeff and Divjot, I agree with them that we will put a blacklist for the three commands in question behind a build flag. We want the Go driver to work like all other drivers for all other users. Changing the spec and changing all other drivers is unnecessary work. It's unfortunate that we have to do this work in the Go driver.

Comment by Jeffrey Yemin [ 20/Nov/19 ]

behackett it's not clear how we would expose a private API for cloud.  What would be the mechanism for it?

Comment by Divjot Arora (Inactive) [ 20/Nov/19 ]

behackett One of my earlier suggestions on the HELP ticket was to add a RunCommand option to opt-out of implicit sessions for a single operation, eliminating any command name checking in the driver. jeff.yemin brought up the fact that this increases our public API surface and would be an option that's not matched in other drivers, so it could be confusing. Happy to revisit if you think that's the right path, though.

Comment by Bernie Hackett [ 20/Nov/19 ]

Can we just add a private API for the cloud team that allows them to run arbitrary commands without an implicit session? No need for a whitelist, no need for build flags, no need for additional parameters to public methods.

Comment by Jeffrey Yemin [ 20/Nov/19 ]

By "smallest change", I'm referring to the application-visible behavior, not the implementation. I agree that C-style ifdefs are easier to manage than Go-style build flags, but you have to work with the tools that you have.   I think if the stakeholders are comfortable with the application-visible behavior aspect of this, we can see what it looks like in a PR and decide whether it's just too much to bear from a maintenance perspective.

behackett would you be comfortable with a spec change or deviation as an alternative?  It feels odd to me to do that to work around a server bug that has been fixed and back-ported already, but I'd like to get your perspective on it. 

For reference, here's divjot.arora's prototype, so everyone can see what we're talking about: https://github.com/divjotarora/mongo-go-driver/commit/5ddae9e4af515f3fe33bf58659a032e236da8062

 

Comment by David Golden [ 20/Nov/19 ]

I would not describe a build flag as the "smallest possible change", given that it requires a separate .go file and some sort of global data structure only populated if the build flag is set. I would hate to see that live forever in the code base to work around this issue.

(If we had C style #ifdef, I wouldn't object to that as a quick fix, but Go isn't like that.)

Comment by Divjot Arora (Inactive) [ 20/Nov/19 ]

Given that a build flag is probably overkill for this, I can see doing this as a SPEC ticket + GODRIVER ticket to automatically exclude implicit sessions on some commands. I definitely don't want to have to enumerate every command that doesn't require an implicit session, as I think this would take too long and has little benefit. That being said, I'm find doing this whatever way is agreed upon by jeff.yemin and david.golden.

Comment by Jeffrey Yemin [ 20/Nov/19 ]

There's probably not a need for an implicit session on most commands.  But that's not the problem we're trying to solve here.  The problem is there is a server bug for these three commands, introduced in 3.6 (3.6.0?) and fixed in a subsequent 3.6 patch (3.6.16?) and in 4.0+.  We are trying to make the smallest change we can to help the automation agent to work around this bug because it has to run against all the affected 3.6 patch releases.  For normal users, we would just tell them to upgrade their server to the latest patch.

Comment by David Golden [ 20/Nov/19 ]

In the description, you said [emphasis mine]: 

If specified during compilation, the driver will not add an implicit session for those commands if they are run through RunCommand

That implies you already have a white list that only gets populated if a build flag is set. I'm suggesting making the white list always active without bothering with the build flag.

Comment by Divjot Arora (Inactive) [ 20/Nov/19 ]

david.golden It's possible we could do this through a larger whitelist but that would require more work to figure out all possible commands that don't require sessions and can be run without being authenticated if a session is not provided (the commands in question are only problematic if the session is included and the user is not authenticated). To me, that feels like more work than it's worth.

Comment by David Golden [ 20/Nov/19 ]

Is there ever any need for an implicit session on those three commands?  None of them are long-running and would need to be killed.  Could we consider whitelisting those specific command without requiring a build flag?  To me it seems like a reasonable spec deviation.  (And if really necessary, it would be a trivial addition to the spec to say "MAY omit session ID" for those commands.)

Comment by Louisa Berger [ 20/Nov/19 ]

LGTM!

Generated at Thu Feb 08 08:36:21 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.