search

docker-archive / oscalkit

NIST OSCAL SDK and CLI
https://docker.github.io/oscalkit/
Other
36 stars 23 forks source link

Revamp project to work with latest OSCAL #88

Closed isimluk closed 4 years ago

isimluk commented 4 years ago

this brings

Original Description (Dec 2019)

This is show case work in progress.

I am interested to learn if this project is interested in revamp or not. Please advise. Thanks!

isimluk commented 4 years ago

/cc @anweiss

anweiss commented 4 years ago

Thanks for this @isimluk! Will take a look and comment back.

anweiss commented 4 years ago

Hey @isimluk sorry for the delay on getting this merged in. I'm waiting on the repo admins to adjust the permissions of the repo so I can proceed with the review. CC @justincormack

isimluk commented 4 years ago

No worries. This is not completed yet.

From the project governance perspective: why did the repo moved from github.com/opencontrol/oscalkit where multiple vendors had access to? It seems that this repo progression / development slowed after the transition. ?

anweiss commented 4 years ago

@isimluk we managed to fix the merging issues so can merge once reviewed and ready. I took a cursory glance at your proposed changes and everything is LGTM thus far. Appreciate some of the cleanup on the go generate templating :) ... let me know once you feel that this PR is in a completed state and I'll review again and merge. Feel free to put the PR into a "Draft" state until then.

As far as project governance, this is still being worked out. I agree with you in that the project would be better served under a more security-/compliance-focused community, whether that be opencontrol or elsewhere. I believe development against this repo has been slow regardless of which org it has been under. So I'm certainly game for anything that could be used to kickstart momentum, especially given the recent OSCAL momentum. But at this point, I have to defer to the folks at Docker in regards to project governance going forward. CC @justincormack

isimluk commented 4 years ago

Thanks @anweiss, will let you know when this is mergeable.

isimluk commented 4 years ago

@anweiss, I think You can merge this any time you see a fit now.

Note, there are failures here and there (since the standard changed). I will continue working on those. We also have to add support for new oscal functionalities. So, it is safe to expect more to come.

anweiss commented 4 years ago

Hey @isimluk, since there still are some commits being pushed up, ping me once you're ready for another review :) excellent work so far! appreciate all the contributions!

isimluk commented 4 years ago

Thank You @anweiss.

I will keep this PR in mergeable state so it can be merged any time. In my mind, this can be merged any time as it represents improvement over what we already have in the repo.

Nevertheless, there is so much work to be done, so I will continue adding patches until it is merged and then I will simply open follow-up PR.

The upside of merging this early is that it may be easier for me to receive early feedback on this work from the greater oscal community.

anweiss commented 4 years ago

@isimluk looks like you're using some new functions introduced in Go 1.12 (e.g. strings.ReplaceAll) ... would you mind bumping the Dockerfile versions to address this so as not to break the Makefile tasks? Then I'll go ahead and merge. Thanks!

isimluk commented 4 years ago

@isimluk looks like you're using some new functions introduced in Go 1.12 (e.g. strings.ReplaceAll) ... would you mind bumping the Dockerfile versions to address this so as not to break the Makefile tasks? Then I'll go ahead and merge. Thanks!

@anweiss, done.

Sorry for the long wait.

anweiss commented 4 years ago

@isimluk looks like you're using some new functions introduced in Go 1.12 (e.g. strings.ReplaceAll) ... would you mind bumping the Dockerfile versions to address this so as not to break the Makefile tasks? Then I'll go ahead and merge. Thanks!

@anweiss, done.

Sorry for the long wait.

Thanks @isimluk. Could you actually bump to Go 1.13 (since go.mod is set for 1.13)? And could you also do this for Dockerfile.build and Dockerfile.generate?

isimluk commented 4 years ago

Thanks @isimluk. Could you actually bump to Go 1.13 (since go.mod is set for 1.13)? And could you also do this for Dockerfile.build and Dockerfile.generate?

Done.

anweiss commented 4 years ago

@isimluk since we don't yet have CI updated to automatically generate the type bindings (which would be ideal per https://github.com/docker/oscalkit/pull/90#issuecomment-575055089), could you add the generated bindings to this PR in the interim?

isimluk commented 4 years ago

@isimluk since we don't yet have CI updated to automatically generate the type bindings (which would be ideal per #90 (comment)), could you add the generated bindings to this PR in the interim?

Last time I checked github actions are not allowed under docker organization.

isimluk commented 4 years ago

@isimluk getting really close to merging. Would you mind also addressing the failing tests? Thanks!

I think tests were failing even before I came. There is lot of scaffolding (like these Dockerfiles) that seems inefficient and only increase complexity. How would You think If I kept my fork growing separately in the mean time to see which parts are useful and which are really not?

anweiss commented 4 years ago

@isimluk since we don't yet have CI updated to automatically generate the type bindings (which would be ideal per #90 (comment)), could you add the generated bindings to this PR in the interim?

Last time I checked github actions are not allowed under docker organization.

Correct. So for now, let's just include the generated bindings in the PR. Or we can open another PR after this is merged with the updated bindings.

anweiss commented 4 years ago

@isimluk getting really close to merging. Would you mind also addressing the failing tests? Thanks!

I think tests were failing even before I came. There is lot of scaffolding (like these Dockerfiles) that seems inefficient and only increase complexity. How would You think If I kept my fork growing separately in the mean time to see which parts are useful and which are really not?

Fair enough. In that case, we'll disregard the failing tests for now until we get a better gauge on things.

isimluk commented 4 years ago

LGTM ... @isimluk let me know if we're good to merge ...

Thanks! We are good to merge. But expect me to come up with more updates in near future.