Adds remote debug capability for secretless broker #1056
Conversation
diverdane
force-pushed
the
mssql-remote-debug
branch
3 times, most recently
from
ea5398f to
fbeea1b
Compare
| secretless-debug: | ||
| build: | ||
| context: ../../../../.. | ||
| dockerfile: Dockerfile.debug | ||
| ports: | ||
| - 2223:2223 | ||
| - 40000:40000 | ||
| security_opt: | ||
| - apparmor:unconfined | ||
| - seccomp:unconfined | ||
| cap_add: | ||
| - SYS_PTRACE | ||
| volumes: | ||
| - ../secretless.yml:/secretless.yml | ||
| depends_on: | ||
| - mssql |
There was a problem hiding this comment.
why can't we just add this image to the test/connector/tcp/mssql docker-compose.yml file and update the README in that dir to include these instructions? it seems not very DRY to me to have a separate dir within the MSSQL test suite for this stuff
There was a problem hiding this comment.
Yes, good point! I moved the secretless-debug docker-compose stuff into the parent directory, and eliminated duplication in the test-local scripts. As for the README, there's no README in the parent. Should I move it up and add some notes for generic secretless testing?
diverdane
changed the title
diverdane
force-pushed
the
mssql-remote-debug
branch
3 times, most recently
from
06c0bfe to
8813947
Compare
diverdane
changed the title
diverdane
force-pushed
the
mssql-remote-debug
branch
2 times, most recently
from
6bbb304 to
ab90077
Compare
go.mod
Outdated
| @@ -24,7 +24,6 @@ require ( | |||
| github.com/joho/godotenv v1.2.0 | |||
| github.com/json-iterator/go v1.1.8 // indirect | |||
| github.com/lib/pq v0.0.0-20180123210206-19c8e9ad0095 | |||
| github.com/modern-go/reflect2 v1.0.1 // indirect | |||
There was a problem hiding this comment.
Any idea why these deps got removed?
There was a problem hiding this comment.
Hmmm... I don't believe that they should be getting removed. The reflect2 dependency should be there because of the github.com/json-iterator/go v1.1.8 // indirect that shows up 2 lines earlier. Coincidentally, this dependency is the one I used as an example for 'git mod why ...' command for the Go Mod section in our Go style guide: https://github.com/conjurinc/docs/pull/139/files#diff-b48d7f4cab90a224552e7e2f355e0a71R329-R337
There was a problem hiding this comment.
My best guess is that these deletions were due to some cruft leftover in my local workspace. To check this, I undid those deletions and forced a build (which should update go.mod as necessary), and those entries stayed in go.mod (as they should). Change set has been updated.
There was a problem hiding this comment.
I'm still seeing them as deleted, but your comment implies this file should not have changed in this PR - can you please clarify?
There was a problem hiding this comment.
After rebase with latest master, this issue goes away.
I think the go command had been "doing the right thing" in removing these dependencies from go mod. In fact, in the latest master, these have actually been deleted. I suspect that someone may have done a prior merge of changes with a go.mod that wasn't quite up-to-date. This could happen, I suppose, if someone does a merge in a workspace where they haven't executed any go commands, or they did 'git checkout go.mod' for some reason before committing. Either way, the go command was trying to update go.mod to what it should be.
diverdane
force-pushed
the
mssql-remote-debug
branch
3 times, most recently
from
e244020 to
62feb40
Compare
| @@ -0,0 +1,47 @@ | |||
| # Using the Secretless-Broker Remote Debug Image | |||
There was a problem hiding this comment.
can we make this just README.md in this directory? for now, it can just include remote debug instructions - but having this here would make it easier for devs to add other mssql-specific dev notes, and that can be quite handy later
There was a problem hiding this comment.
I think with that these changes could go in
There was a problem hiding this comment.
Okay, I moved the remote-debug.md to README.md, and added an intro into the README.md to describe what remote debugging is.
diverdane
force-pushed
the
mssql-remote-debug
branch
3 times, most recently
from
383905c to
2055456
Compare
WIth this change, a Delve-capable IDE such as Intellij/Goland can attach to a running secretless broker container, allowing breakpoints to be set, single-step debugging, etc. The remote debug capability will be helpful for debugging integration tests. (Unit tests, on the other hand, can already be easily debugged directly from the source code without remotely connecting to into a container). This change includes a new "debug" image for the secretless broker that adds a Delve binary (to allow remote debug connections between the secretless broker and an IDE), and compiles the secretless broker with code optimizations disabled (for better debugging experience). To use remote debugging on the secretless broker, follow instructions in the 'Debugging Secretless Broker as it is Running in a Container' section of the test/connector/mssql/README.md file.
diverdane
force-pushed
the
mssql-remote-debug
branch
from
2055456 to
ec22a81
Compare
What does this PR do (include background context, if relevant)?
(LOW PRIORITY)
This change adds remote debug capability for the secretless broker.
WIth this change, a Delve-capable IDE such as Intellij/Goland can attach
to a running secretless broker container, allowing breakpoints to be set,
single-step debugging, etc.
The remote debug capability will be helpful for debugging integration tests.
(Unit tests, on the other hand, can already be easily debugged directly from
the source code without remotely connecting to into a container).
This change includes a new "debug" image for the secretless broker that
adds a Delve binary (to allow remote debug connections between the secretless
broker and an IDE), and compiles the secretless broker with code optimizations
disabled (for better debugging experience).
To use remote debugging on the secretless broker, follow instructions
in the test/connector/mssql/README.md file.
What ticket does this PR close?
Connected to #[relevant GitHub issues, eg 76]
Where should the reviewer start?
What is the status of the manual tests?
Have you run the following manual tests to verify existing functionality continues to function as expected?
If this feature does not have any/sufficent automated tests, have you created/updated a folder in
test/manualthat includes:startandstopscripts to spin up and tear down the test environmentstestscript to run some basic manual tests (optional; if does not exist, the README should have detailed instructions)Links to open issues for related automated integration and unit tests
Links to open issues for related documentation (in READMEs, docs, etc)
Screenshots (if appropriate)