kobaltcore / renconstruct

A utility script to automatically build Ren'Py applications for multiple platforms.
MIT License
7 stars 2 forks source link

GItHub Action Support Advice #1

Closed devorbitus closed 3 years ago

devorbitus commented 3 years ago

I would like to create a GitHub Action that utilizes renConstruct to build distributions from GitHub. Right now, renConstruct will utilize an existing cache directory which I can provide through the cache GitHub action. However, the first time renConstruct is run after downloading the SDK and raft, it generates a new android Keystore without giving the opportunity to override.

I was thinking I could submit a PR to renConstruct that would allow an additional non-required option that would allow an additional path to search for tasks. The GitHub action's container would have an additional task inside like this...


### Logging ###
import os
import sys
import base64
from renconstruct import logger

class OverrideAndroidKeystoreTask():

    PRIORITY = 1

    def __init__(self, name, config):
        self.name = name
        self.config = config

    @staticmethod
    def validate_config(config):
        pass

    def pre_build(self):
        logger.info('Time to do the prebuild of OverrideAndroidKeystoreTask')
        base64androidKeyfile = os.environ.get('BASE64_ANDROID_KEYSTORE_OVERRIDE','')
        if not (base64androidKeyfile and base64androidKeyfile.strip()):
            logger.error('Environment variable of BASE64_ANDROID_KEYSTORE_OVERRIDE holding the Base64 encoded Android Keystore was not found!')
            sys.exit(1)
        else:
            base64androidKeyfileDecoded = base64.b64decode(base64androidKeyfile)
            sdkVersion = self.config['renutil']['version']
            ks = open(f'./cache/{sdkVersion}/rapt/android.keystore','w')
            ks.write(base64androidKeyfileDecoded)
            ks.close()
        pass

    def post_build(self):
        pass

Then the command to execute renConstruct would be something like this...

renconstruct -d -i ../renpy-project -o ../renpy-project_dist -c ../renpy-project/config.yml -a ./additional-tasks

and that config.yml file would look something like this...

tasks:
  set_extended_memory_limit: false
  notarize: false
  clean: true
  patch: false
  override_android_keystore: true

patch:
  path: "renpy-patches"

build:
  win: true
  mac: true
  android: true

renutil:
  version: "7.4.5"
  registry: "cache"

renotize:
  apple_id: fill-this-in
  password: fill-this-in
  identity: fill-this-in
  bundle: com.my-organization.my-game
  altool_extra:

There would be a GitHub secret added that would have the base64 encoded android.keystore file stored inside it and executed like this...

- name: Build Ren'Py-based Visual Novel distributions using renConstruct
  uses: devorbitus/renconstruct-build-action@master
  with:
    sdk-version: '7.4.5'
    project-dir: '.'
    config: 'config.yml'
  env:
    BASE64_ANDROID_KEYSTORE_OVERRIDE: ${{ secrets.ANDROID_KEYSTORE }}

Do you think this approach is the right way to go or would it be better to make changes to renutil to allow for android keystore overrides or another suggestion I have yet to consider?

devorbitus commented 3 years ago

My thinking with going with a task is that the project still has to enable it in order for it to be executed within the config.yml file but they don't have to have the code of the task inside their project and it will be completely skipped if they don't enable the task.

devorbitus commented 3 years ago

Another option I just thought of is to actually add the override android keystore task as an actual task inside renConstruct but set to not run by default and instead have an option config for the contents of the base64 encoded android keystore file value rather than an environment variable

kobaltcore commented 3 years ago

Being able to set an existing Android keystore easily is certainly a thing that renconstruct should support properly such that CI builds become possible for the Android platform.

Since renutil's scope is purely that of installing Ren'Py in whatever version one desires and making sure that it runs, I think it is more reasonable to keep keystore management in the "build" solution (renconstruct), not the "run" solution (renutil).

I think a hybrid approach which is an amalgamation of the ideas you have already proposed is the most sensible:

The second part is especially important because generally, you will most likely commit your renconstruct config file to your repository, so including your keystore file in it would be a very bad idea. In such cases, the CI should be able to store your keystore in an encrypted fashion and only avail it to the actual CI job that requires it. GitHub actions support this since you can mount encrypted secrets as environment variables. I know that GitLab CI can also use protected/masked environment variables for such purposes. This would prevent one from accidentally leaking their keystore file.

On the other hand, if you're only building locally, you may not want to bother with environment variables during the build process, so adding the keystore to the config file would be an easy way to supply it without doing any extra work.

As such I would propose that we support both, such that:

One extra thing to consider for "stateful" executions of renconstruct (such as repeatedly running it on your local machine) would be what happens when you specify a custom keystore during a build and remove it during the next. Having overwritten the original one, we'd still build with the one you specified in the last build, even though this is no longer explicitly requested.

A solution to this could be enhancing the new task so it backs up the original, fresh keystore and reuses it when nothing else is supplied.

devorbitus commented 3 years ago

I agree, I was looking at something like this as a way to access the keystore inside GitHub https://stefma.medium.com/how-to-store-a-android-keystore-safely-on-github-actions-f0cef9413784

Let me create a new PR and see what we think with this new guidance.

kobaltcore commented 3 years ago

Indeed, storing the keystore as base64 in an encrypted environment variable seems workable to me.

Regarding the implementation, I'm currently dreaming up a somewhat more invasive way to implement this keystore override task while allowing for repeatable builds (such that the original keystore can be restored when necessary). This will require a few modifications to renconstruct itself, so there may be some commits coming in once I get a proof of concept implementation working.

I'm also strongly considering building some template actions for GitHub's action system so people don't have to write 'em themselves, so that may also be in the pipeline.

kobaltcore commented 3 years ago

Okay, I've added a first tentative implementation for the keystore overwrite task in 60679c389b78c2660de52cab533028e5255e7666.

This task is disabled by default (like any other task that is not explicitly enabled) and can pull keystore data in a base64-encoded form from either the overwrite_keystore.keystore field in the config file, or from the RC_KEYSTORE environment variable. It will validate the keystore data by decoding it (and checking for the existence of at least one of the two options) when it is enabled and first loaded to make sure we have a modicum of reliability before the actual build process starts.

I have yet to test this fully, so there may be some bug fixes coming later down the line.

Some documentation has been added for this task as well right here.

devorbitus commented 3 years ago

60679c3 means this issue can be closed, thanks!