New script to set up a clone for testing against the security repo.

[dmonllao]

Hi Fred,

I am not sure if you have access, but we discussed this proposal in https://tracker.moodle.org/browse/HQ-1429.

To resume a bit the PR; in a near future we will need minor release weeks' testing against the security repository instead of against integration.git. One of the advantages that we seek moving security issues to a security repository is to prevent security patches to be released to the public days before a minor release. Even if testing happens against security.git it is likely that at some point we (devs) make mistakes and push security patches to github; one of possible scenarios is that I am the assignee of an issue that is failing and I need to patch it. We should prevent devs pushing those branches to github and instead upload patch files to tracker.

[FMCorz]

Hi David,

This script seems like a workaround rather that adding the functionality to MDK itself. You're removing all the remotes, includes the ones that MDK would expect. Beside, it seems that this script relies as much on the dev running it than the dev not pushing branches. Isn't the idea to remove the human factor?

Did you notice that mdk push does not allow pushes to github for security issues? That is done by calling Jira. In your case, all you'd need to do is checking whether the repo qualifies as a "security" repo, and reject the push (or send a patch to Jira). That doesn't cover git push though.

Here are a few suggestions:

• Support security repos in MDK (e.g. mdk create --type testing)
• Have MDK to add a bunch of hooks on create
• The hooks only contain a call to MDK which takes over ** This allows many other things to be done in MDK, such as prefilling commit message or any custom "hook" for Moodle instances
• mdk doctor to add the missing hooks

What do you think? Is there urgency in merging this script?

Note that anyone can install custom scripts by adding them to ~/.moodle-sdk/scripts.

[dmonllao]

Hi,

Unfortunately we can never get rid of the human factor :P This tries to minimise what they can do by setting the testing environment once (they can still delete the hook and set up everything they want).

Yeah, you told us that mdk push prevents devs from pushing security issues, but (for what it seems) it does it by checking the branch name (https://github.com/FMCorz/mdk/blob/master/mdk/commands/push.py#L150) and as explained above this does not solve the main reason why we need this: Minors release week people will test all issues using security.git repo, my issue (not a security issue) fails so I prepare a patch on top of the current testing branch and I push it to github, along with all patches currently in security.git.

My first approach was a new type as you suggest, but I stopped when I saw that would require a major refactoring as even -t option alias is set to --integration and all the code seems to deal with stable or integration. I am not sure if I understand your suggestion to add a bunch of hooks on create or mdk doctor, would that be possible without --type testing? I guess we would need a way to identify that instance as security testing. All these difficulties is what made us think that a simple script would be easier than refactoring.

There is no urgency, we will be quite busy until 3.3 is released. Being this important for all Moodle HQ users we understand that it would ideally have a space in mdk code. If we can not agree on any solution we will provide a separate script.

[FMCorz]

You've got a point, mdk push only prevents pushes done via MDK, and only based on the current branch. So yes, there needs to be a flag identifying a repository as being a "security" repo and block pushes from there. Supporting security repos is a major refactor, I agree.

I mentioned doctor because if you add hooks on create, you probably want to add hooks on instances created prior to the patch as well, and doctor is the right place to do these things.

I understand the need for a hack, though could you consider the following:

• Adding some comments at the top of the script explaining its purpose.
• Double checking that this works on MacOS (I've had Bash scripts compatibility issues before)
• Using mdk config show remotes.security to get the URL to the security repo.
• Keep all remotes, except for origin (Note that origin may have another name, see mdk config show upstreamRemote)
• Set the security repo as the origin to enable mdk update

As the hook is in place, other remotes shouldn't matter.

With that done, gradual work to identify a "security" repo from within MDK can be done.

[dmonllao]

Thanks Fred, I will do the proposed changes.

[dmonllao]

I've pushed a new version of the patch including the requested changes.

[FMCorz]

Thanks David. I added a commit to chmod +x the script as otherwise it doesn't work. I also added the usage of an absolute path to the hook file. I also echoed the error out to stderr so that the error appears when using mdk push. Cheers.