-
Notifications
You must be signed in to change notification settings - Fork 74
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
Nancy uses Nexus IQ Server, the journey #68
Conversation
|
||
`build, develop, stage-release, release` | ||
|
||
By default `-stage` will be `develop`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would the default be develop
?? Im going to guess 99% of the time this will be running on CI so shouldn't it be build
. Keep in mind i have no real idea what this stages mean in IQ (only briefly read some of the docs :) ) so I'll defer to you of course.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have build
> develop
> release-stage
> release
. I went with develop as a default as Nancy can be used by developers locally pretty easy. We could set it to build
for sure, but it kinda depends on how you setup your policy TBH with Nexus IQ. I originally had it as build
ftr. I can go either way!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool cool.... that makes sense then.
#scopeCreepAndMaybeNotWorthIt
But would it make sense to detect if on CI (looking for TRAVIS, CIRCLECI, CI, etc etc) environment variables and if one of those is set you change the stage automagically to build
/release-stage
?? Maybe that is still back into the "how you setup your policy in Nexus IQ" again and it doesn't matter. ¯_(ツ)_/¯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I LIKE THAT. We'd still need to allow someone to override it, though, because people use Travis, CircleCI etc... to do CD too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah for sure...if you pass it in then that is the value to be used. But nancy will attempt to "smart default" it for you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nancy is so clever!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should create a ticket for this, it was a fantastic suggestion, however I dunno if I want to do it in this PR.
I assume you saw the tests are broken....looks like you can't run it via passing go.sum or Gopkg.lock anymore ¯_(ツ)_/¯ Also tests are a lacking in this PR but probably wouldn't hurt to add at least something around the cyclonedx stuff and maybe something around the api calls into IQ |
@zendern yeah, I saw all that, gonna fix all that up tomorrow, just wanted to get some early feedback on my approach! |
Re: squash into master. Nah that’s fine if you want to just do it that way. 👍 |
Bit of a follow up to, I'm noticing it's hard to test error cases in Nancy because we throw around |
@DarthHater Latest changes look good to me. I do have a concern using panic and the fact that it now muddies user messaging when it is encountered. Basically it means two things....we lose control over the exit code coming back (maybe this is fine as long as it not 0 its fine) and we now give the user a stacktrace. Which sure nancy is a dev tool but feels like the wrong thing to show a user, most if any will not care about nancy internals. So few options come to mind....
|
@zendern I'm with you on the stack trace stuff.
I'm just trying to get more towards that, using panic in libs a bit more and allowing someone to recover if they want to, rather than explicitly force them to os.Exit |
@DarthHater so then you plan is to put that into place?? So somewhere probably on main.go we capture the panic and return the message/correct error code |
Well, I'm kinda stuck on it personally. The middle ground is probably to panic, and then recover by throwing an error from our public APIs, that's consistent with the golang principle anyways. More so having an open conversation so I can figure out how to wrap this PR up, while heading some sort of direction :) |
@DarthHater sooooo middle ground maybe to get this pushed along. The panic stuff you are testing error states right?? What if we go back to exit, scrap the test you wrote around error path and write up an issue to move the project from exit -> panic and implement the public error handler code that would be needed. Eat the tech debt around error handling testing in this case and get this shipped ¯_(ツ)_/¯ |
That sounds good, bud. I'll see what I can move forward today and then re-tag you in here! |
@zendern , moved back to errs and exits for now, if you wanna give this ole PR the rubber stamp! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^^^ @DarthHater don't make me approve again cause its not possible.
This is a quick lil ditty to get Nancy to work with Nexus IQ Server
This pull request makes the following changes:
iq.go
that handles communication with Nexus IQcyclonedx
package that will turn a[]packageurl.PackageURL
of purls into a minimal SBOM string./nancy iq -a application
Successful output looks like:
Draft PR for now so we people can laugh at my implementation with less shame. I'll add README results after we sort what I did technically.
cc @bhamail / @zendern / @ken-duck / @ajbrown / @fitzoh