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

Add GitHub Action workflow to validate/format Rock-ons #348

Closed

Conversation

StephenBrown2
Copy link
Contributor

@StephenBrown2 StephenBrown2 commented Jul 19, 2023

Greetings again, I've got a new toy for you to try. I'd been wanting some way to make sure that rock-ons were formatted in a similar way, and that everything was present that needed to be, and nothing that wasn't. So I wrote a tool in my new primary language: Go. You can find the repo here: https://github.com/StephenBrown2/rockon-validator but I would just as well have it live under the rockstor org. The transfer has been made and the repository is now at https://github.com/rockstor/rockon-validator

General information on project

Options

I gave it a couple options, namely --check, to simply exit non-zero if files need changing, --diff, to output the changes that would need to be made to stdout, and --write, to actually overwrite the files with proper formatting and validation.

GitHub Action

I've added it as a GitHub Action for this repo so we can see how it works. Currently almost every file would need changes, which I can make in a different PR or this one, though a demonstration does exist in the README for the tool.

The GitHub action will run the tool over every file in the repo, and suggest changes based on what it would have overwritten. We could potentially add it as a pre-commit script or requirement in the Checklist that it be run before a Rock-on PR is accepted.

Features

Currently, the tool does the following:

  • Parses the rock-on file(s) and ensures that it meets the expected model.
  • Checks that an entry for the file exists in the root.json file, assumed to be in the same directory as the file, or another path can be given.
  • Checks that the name for the Rock-on matches between the rock-on.json and the root.json, and warns if they don't. I wasn't sure if we wanted to auto-correct so they match or in what direction if we did, since I think it might mess up the listing for installed rock-ons if the name changed?
  • Sorts all objects by the order in which declared (i.e. as listed in the registry README), and maps by their keys. This is a function of how the go JSON marshaller works, and cannot easily be modified. However, since we want root.json sorted alphabetically anyways (surprise, it isn't currently), I figured this was a reasonable trade-off and would enforce a black-like formatting for the JSON. NOTE: This also means that the "environment" map sorts by the environment variable name, rather than the "index" value, as they are currently sorted.

Other Issues

While working on it, I noticed a couple things were not documented in the README, namely container_links and custom_config, but I've based the model off of what currently exists in the registry JSON, rather than trying to figure out what is an expected field and what isn't from the rock-on model.

Future Enhancements

Things the validator does not do, but might be able to:

  • check the "description" string for the docker image used
  • validate that the "website" string is a valid URL

Fixes #91

Checklist

  • Passes JSONlint validation
  • Entry added to root.json in alphabetical order (for new rock-on only)
  • "description" object lists and links to the docker image used
  • "description" object provides information on the image's particularities (advantage over another existing rock-on for the same project, for instance)
  • "website" object links to project's main website

@Hooverdan96
Copy link
Member

@StephenBrown2 this looks pretty cool!

@phillxnet
Copy link
Member

@StephenBrown2 This is really nice. Well done. Go you say: look at you! I've been meaning to familiarise myself with go for quite a while now so this is quite intriguing.

but I would just as well have it live under the rockstor org.

Yes I think that would be preferred, and if it is to be integrated into our workflow it would have to be really. So if you would be good with that, that would be dandy. Which brings up the license question: rockstor-core is GPL-3.0-or-later, but our Rock-on definitions are AGPL-3.0-or-later. However I'm unfamiliar with go, does its runtime have additional license burdens?

See: https://github.com/rockstor/rockstor-core#readme

@FroggyFlox & @Hooverdan96 the function here re GitHub Actions & Rock-ons is more in both of your areas as it goes so I'll mark for review and have to leave that review to either/both of you I'm afraid.

But if we are to integrate this rather nice mechanism it must have a license, as all our stuff has to be licensed, that is:

FSF Free/Libre & OSI approved

It's a requirement of our Open Collective and down to the nature of the project as a whole. And we have now standardised on https://spdx.org/licenses/ licensing identifiers as they are machine readable. So no license is a deal breaker for inclusion of any type, and a non FSF Free/Libre & OSI approved license is also a no go: as it goes.

Excellent work this thought and something we would really use.
Thanks for submitting this.
If the License thing could be worked out - and it could be maintained under the Org, likely under the exact name you have used: "rockon-validator" to match its target repo of rockon-registry, the this could be quite the addition to our workflow. And @FroggyFlox has done some more GitHub Actions work of late so this is actually quite timely, if we can iron out the above points. If an acceptable license could be agreed and the org hosting is OK all around: I or @FroggyFlox could create the new Org repo and this could be submitted to that repo for review, possibly.

Thoughts. It would be great to have a validator - if licenses and org hosting allow that is.

@StephenBrown2
Copy link
Contributor Author

StephenBrown2 commented Jul 19, 2023

Which brings up the license question: rockstor-core is GPL-3.0-or-later, but our Rock-on definitions are AGPL-3.0-or-later. However I'm unfamiliar with go, does its runtime have additional license burdens?

I don't know if it is a burden, but the Go source files are distributed under the BSD-style license found in the LICENSE file. There are a couple libraries in use which either use the above or MIT licenses.

I'll happily license it as GPL-3.0-or-later, as I don't believe it would become a network service and thus require AGPL licensing. Feel free to suggest otherwise, I'll use whichever is easier for the project to use.

@phillxnet
Copy link
Member

@StephenBrown2

the Go source files are distributed under the BSD-style license found in the LICENSE file.

OK, from that license link we have that as a:
'BSD 3-Clause "New" or "Revised" License'
which in SPDX from my prior link would be: "BSD-3-Clause", see: https://spdx.org/licenses/BSD-3-Clause.html

Which is both FSF Free/Libre & OSI approved so we are good on the non-encumbered front re bundled run-times (the go interpreter presumably).

There are a couple libraries in use which either use the above or MIT licenses.

MIT license version may be important: i.e. The Apple MIT is neither FSF Free/Libre or OSI approved (who would have thought).

The plain SPDX 'MIT' is the only one that is both FSF Free/Libre & OSI approved: https://spdx.org/licenses/MIT.html

All other MIT variants listed on that SPDX link: https://spdx.org/licenses/ are neither or only one side approved.

I know it's a drag but our licensing position is super important and I for one cannot contribute, or host as the maintainer under an Open Collective, if it's not at least clear what we are intending to use with all due diligence to clarify what we are building with. Your program, as a hosted service scirts many requirements: but we may one day want to have it as a bundled option: or as a Rock-on that we host for example. To assist folks before they submit to the rockon-registry. Just a thought.

Thanks for persevering with all this. It just took me months to get all our licensing clear and expressed everywhere in SPDX approved notation and I'd hate to back-slide and risk the entire project's new Open Collective standing.

I'll happily license it as GPL-3.0-or-later, as I don't believe it would become a network service and thus require AGPL licensing.

Excellent, thanks for the flexibility on that front. However: as a GitHub Action surely it would be the definition of a network service no? I guess those outside the Org are not accessing it via an API: but the Org would be. Licensing is so exciting is it not!

@FroggyFlox @Hooverdan96
Given @StephenBrown2 is game to license as per our other repos and in accordance with our existing licenses (give we establish the Go libraries used MIT version/s is similarly so) shall we create a repo for this rather existing addition? This will of course depend upon that MIT version question.

@StephenBrown2
Copy link
Contributor Author

StephenBrown2 commented Jul 20, 2023

Go, being a statically compiled language, doesn't have an "interpreter", so that should easy.

The library in question (which is strictly to color the logs and can totally be removed if needed), appears to use the "plain" MIT license. TBH I didn't even know there were multiple MIT licenses.

I think we are in the clear to use the GPLv3+, but I am still curious about the idea of the CLI script being a "hosted service". It's GitHub Actions that's the service, the CLI is just that: a CLI tool, intended for use as you said to assist folks before they submit to the rockon-registry (If approved, I would note it in the README as a step for the contributor to run before even opening a PR). It has no network interface (AGPL applies primarily to software that is used to provide service over a network) and the AGPL states:

The GNU Affero General Public License [...] requires the operator of a network server to provide the source code of the modified version running there to the users of that server.

So, in fact, I would think that the licenses should be flipped; in that rockstor-core would need the AGPL, as being primarily (only?) available as a network server, and the rockon-registry, being only useful if the actual source is available, being under the GPL. Or, since they are not "code" or even close to a "service", but rather "configuration", a simpler license such as Creative Commons, or the zlib license (which is, in fact, both FSF Free/Libre & OSI approved!) would be more appropriate.

Boy, licensing sure is fun! :crossed-eyes:

@Hooverdan96
Copy link
Member

Hooverdan96 commented Jul 20, 2023

Given @StephenBrown2 is game to license as per our other repos and in accordance with our existing licenses (give we establish the Go libraries used MIT version/s is similarly so) shall we create a repo for this rather existing addition? This will of course depend upon that MIT version question.

I think it's a great idea, if/when all of the licensing hurdles are out of the way.

@phillxnet
Copy link
Member

phillxnet commented Jul 20, 2023

@StephenBrown2
Re:

... as being primarily (only?) available as a network server ...

We do not host rockstor - it is self hosted. But we do distribute it. But interesting point yes : however we are way into our existing license, so I think we are pretty much set there. Especially with so many contributors etc. However I'd really like to drop our own dependency count - but that is, as always, ongoing :).

Or, since they are not "code" or even close to a "service", but rather "configuration" ...

Ohh we are getting into some strange epistemological territory there :) . What is code if it is not a configuration (read data) to be interpreted/compiled via a syntax that includes a grammar (making it a language). Just a thought.

The main reason to have our Rock-on configs under AGPL is so that if they are hosted by others - they are obliged to contribute modifications back to the community at large. As they would be making them available via a network: i.e. the internet.

I hope we are done this this license chit-chat :) it really isn't my favourite part of things: but it is critical to the survival of open source projects and for folks to share/benefit from whatever improvements have been made: i.e see OSX or whatever it's call now, vs BSD that it was branched from. One has benefited from the other - but not the other way around. Ergo my lack of enthusiasm for the slacker licenses.

Our docs are Creative commons by the way. But anything code (I know!!) is not recommended to be under Creative Commons. I got that impression of late from an article or podcast that I'm unfortunately unable to resource. Sorry.

Good to have you back contributing by the way.

@StephenBrown2
Copy link
Contributor Author

StephenBrown2 commented Jul 20, 2023

We do not host rockstor - it is self hosted. But we do distribute it.

Yes, but the purpose of the license is not to restrict the producer, but rather anyone that might take the code and offer a derivative, for example if I were to offer Rockstor instances as a hosted service in my hypothetical data center... But I don't mean to drag the conversation on.

While I disagree with some of the interpretations you've made of the licenses, IANAL and have no need to discuss it further. You'll notice I've added the appropriate LICENSE, and am ready to transfer the repository to rockstor the whenever you can give me access to, or however you'd like to handle that. I would of course like to continue working on it based on feedback that I anticipate will arise.

Speaking of said feedback, shall I open another PR here with the formatting changes from running the validator now?

Good to have you back contributing by the way.

Oh, and thanks!

@phillxnet
Copy link
Member

@StephenBrown2
Re:

Speaking of said feedback, shall I open another PR here with the formatting changes from running the validator now?

No lets have this all done as issue/pull-requests sets once it's in the org repo. And on that point.

I'll open a new repo and you can then do a pull request to that repository in the org: if that works for you. You will then already have your fork (original) of it to do pull requests from. Hopefully that makes sense, getting a little later in the day here now.

Thanks for your flexibility and understanding on all this by the way. I agree with the point re rockstor-core, of course, but we are a long-in-the-tooth project on that front. Our Rock-ons are a relatively new capability as it goes. And changing licenses can be quite the ordeal once code is already release under an initial license.

Cheers. Keep an eye out for the new repo - assuming @FroggyFlox is also good with this approach.

@StephenBrown2
Copy link
Contributor Author

Hey @FroggyFlox, I would prefer to transfer (rather than fake-fork as @phillxnet suggested) the repository, as I think that will be the cleanest method. You would need to give me permissions to create repositories briefly while I did the transfer, and then you can revoke that permission once it's done, since I would be added as a collaborator after the transfer.

Information from here: https://docs.github.com/en/repositories/creating-and-managing-repositories/transferring-a-repository

  • To transfer a repository that you own to an organization, you must have permission to create a repository in the target organization.
  • The target account must not have a repository with the same name, or a fork in the same network.
  • The original owner of the repository is added as a collaborator on the transferred repository. Other collaborators to the transferred repository remain intact.

image

Ready to hit the button, as soon as you say go!

@phillxnet
Copy link
Member

@StephenBrown2 I'm really not happy giving repo creation access to an org that I am the owner on. What is wrong with populating vai a pull request, you have no issues pull requests collaborators/contributors. And would you not be a contributor (the only one likely) post the initial meger. What am I missing.

There is no fake-fork: we create a repo in org and you populate it without gaining admin in the org: even for a millisecond. I'm likely missing something here sorry. But we cannot give org rights away willy nilly. Some cannot there-after be revoked you understand. We are kind of in a delicate position here: and as Org owners we state what we are happy with: I'm not happy given repo create access I'm afraid: that seems like a little bit of a jump from where we are now no?

@StephenBrown2

This comment was marked as outdated.

@StephenBrown2
Copy link
Contributor Author

StephenBrown2 commented Jul 20, 2023

Oh! I just had another idea. If @FroggyFlox is handling the repository management anyways, I could simply transfer the repo to them, and they can transfer it to the org. If that is acceptable, go ahead and confirm the transfer email that was just sent to your (@FroggyFlox's) account, and no permissions or roles need changing at all!

Apologies for the long-winded comment above, I'll admit I had only thought of the two possibilities, and not considered the indirect but permission-less option.

@phillxnet
Copy link
Member

@StephenBrown2 Re:

If @FroggyFlox is handling the repository management anyways, I could simply transfer the repo to them, and they can transfer it to the org.

That's a good idea. I'd not thought of that. We have only created new repos in the org and have repo create disabled even for member. The org is used less as a work place and more as a central repository, with folks working-in & committing from their own forks via pull requests only, non of us direct push to org repos unless it's an emergency. My apologies for my tone in my last post: it had been a long day; and org permissions are somewhat of a worry for me as it goes.

Thanks again for helping out here. I'll have to embark on my long planed Go pick-up soon then :).

@FroggyFlox
Copy link
Member

Oh! I just had another idea. If @FroggyFlox is handling the repository management anyways, I could simply transfer the repo to them, and they can transfer it to the org. If that is acceptable, go ahead and confirm the transfer email that was just sent to your (@FroggyFlox's) account, and no permissions or roles need changing at all!

Hi @StephenBrown2, sorry for taking so long to get back to you... my schedule is a bit erratic at the moment.
I just proceeded with both transfers before the original request expired.

Thanks a lot for all of this, by the way! I'll share more elaborate feedback as soon as I have a bit more time (hopefully later today, Eastern time). Sorry for such a rushed input on my end for the moment.

@StephenBrown2
Copy link
Contributor Author

No worries on the time @FroggyFlox , I had a busy weekend myself. Thanks for taking care of the transfers!

@phillxnet
Copy link
Member

Closing as pending new repo developemnt.

@phillxnet phillxnet closed this Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatic validaton of PRs
4 participants