Skip to content

Conversation

@mikeller
Copy link
Member

Fixes #1685.

Based on #2574, will need a rebase after this has been merged.

Included:

  • added support for debug builds for android, producing an APK file for local installation;
  • changed the release build for android to produce an AAB file for Play store upload;
  • changed the release build for android to use a private keystore;
  • changed the release build for android to ask for the version name to be used, to work around the limitations on versioning applications in the Google Play store.

name: pkg.name,
version: pkg.version.replace(regex, '_'), // RPM does not like release candidate versions
name: metadata.name,
version: metadata.version.replace(NAME_REGEX, '_'), // RPM does not like release candidate versions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment still needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't heard anything about dashes ('-') now being supported in RPM version names. Have you?

Copy link
Contributor

@DusKing1 DusKing1 Aug 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I misunderstood this comment that I think this RPM is "Revolutions Per minute", sorry. It's the Red-Hat Package Manager, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Welcome to TLA hell. 😁

@mikeller mikeller force-pushed the add_play_store_build branch from d81a101 to 025b0bd Compare August 25, 2021 03:51
- script: yarn install
displayName: 'Run yarn install'
- script: yarn gulp release --android
- script: yarn gulp debug-release --android
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we need to have both here, debug and non debug build like in Windows:

    - script: yarn gulp release --android
      displayName: 'Run yarn release for android'
      condition: and(succeeded(), eq('${{ parameters.releaseBuild }}', true))
    - script: yarn gulp debug-release --android
      displayName: 'Run yarn debug release for android'
      condition: and(succeeded(), eq('${{ parameters.releaseBuild }}', false))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that part of the build of the android application bundle for the Google Play store is to sign the bundle with a private key. This cannot easily moved to azure without compromising the key, so I have opted to be doing this on a physical machine for now.
There are some more constraints with the Google Play store around versioning that make it hard to fully automate builds to be published in this way, so doing this manually and locally is probably best anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don' t understand exactly... I understand the part that we need a private key, but I don't understand why we can't have the two options (release and debug-release) like in windows. I understand that by default it will execute the debug-release anyway.
I suppose that what you say is that the release will not work because it simply does not have the private key so is better to not put the option here?
If that is the reason, then what we do is to use different private keys for the "official" release and the "official" nightly, what does not seem very clean. The user will need to remove one to install the other. There must be a way to use, in a safe way, the same private key for both versions. Another thing is a local build of the developer, in this case of course the private key will be different only for testing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@McGiverGim: Not sure what you mean with your statements about using different private keys - this is not how the Google Play store works. There will never be a way to install the nightlies through the Play store, so users will always have to overrride security and allow the installation of untrusted packages.
Actually, one thing that this pull request proposes is to use a different package id (com.betaflight.betaflight_configurator_debug vs. com.betaflight.betaflight_configurator) for the nightly / debug builds. Since this is used as the unique identifier for packages in android it will be possible to have the official Play store version and a debug version installed in parallel.

@mikeller mikeller force-pushed the add_play_store_build branch 4 times, most recently from 5f63990 to c3c1b03 Compare August 25, 2021 06:35
Copy link
Member

@McGiverGim McGiverGim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the cordova/release.keystore contain the real private key for signing the final apk? It does not seem very secure to publish the file here then.

@McGiverGim
Copy link
Member

I have been looking how others Android apps do this... for example here, is with a Github Action, but I suppose Azure will have something very similar:
This is a relase.xml workflow: https://github.com/home-assistant/android/blob/master/.github/workflows/release.yml , inside it, it takes the keystore and password from the github secrets and calls this action to inflate it: https://github.com/home-assistant/android/blob/master/.github/actions/inflate-secrets/action.yml In this way you need to upload the keystore and password to the secrets, but at least is not published in the public github repo.

@mikeller mikeller force-pushed the add_play_store_build branch from c3c1b03 to 0ccf28b Compare August 25, 2021 07:42
@mikeller
Copy link
Member Author

mikeller commented Aug 25, 2021

@McGiverGim:

Does the cordova/release.keystore contain the real private key for signing the final apk? It does not seem very secure to publish the file here then.

No, releases use bundle.keystore, which isn't checked in:

https://github.com/betaflight/betaflight-configurator/pull/2577/files#diff-7d841060297ed1d22543caa294064687c58a2a38cf9f13b809d64b61db0a5db7R11

What value do you see in getting the application bundle that Google uses to build the application that are distributed through the Google Play store built in the cloud - there is no way for a user to verify that his application was not tampered with anyway.

@mikeller mikeller force-pushed the add_play_store_build branch from 0ccf28b to 5cc28aa Compare August 25, 2021 09:06
@McGiverGim
Copy link
Member

What value do you see in getting the application bundle that Google uses to build the application that are distributed through the Google Play store built in the cloud - there is no way for a user to verify that his application was not tampered with anyway.

Not for the final version. A lot of projects automate the release, but I understand that we do the release manually, and I'm not against it. But I think it can be valuable to push the nightly version. The user can decide if they want to be subscribed to the beta channel of the app, or only to the release channel. If subscribed to the beta, I think we can publish, in an automated way, the nightly. Is a good way to detect problems and bugs soon. In this case, is good to let the user change between the beta channel and the release channel in a clean way, and I suppose that this will need to use the same key for both (maybe I'm wrong, I never worked with the Play Store).
The protection against tampered comes with the keystore and the Play Store. The developer is responsible to have the private key in a secure place, and the user must install it only from the Play Store. If both things are accomplished the apk is secure and not tampered.
Another thing are developers or similar, we will build and install it using the keystore provided (or another one generated by us).

@mergify
Copy link

mergify bot commented Aug 25, 2021

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

@mikeller
Copy link
Member Author

@McGiverGim: Ah, now I understand, what you are looking for is the automatic publication of nightly builds to the Google Play store. This is not what this pull request is aiming for - this is only to get to the point where we can build relase versions that can be manually published to the Google Play store.
I think this is a reasonable next step, and will make the android version of configutrator available to a lot of users in an easy way.
I am not sure that making unstable development versions of configurator available in the Play store in a way that automatically pushes the latest build is even desirable - as a developer, when testing (which is the target audience for unstable development versions), I would prefer to specifically install the version I want to test with, which is a lot easier if I can just download and locally install the .apk.
But if we think there is value to make it possible for users to get unstable development versions of configurator from the Play store we can certainly consider putting in the effort to make this possible at a later stage - I just don't think there is a lot of value in it.

@haslinghuis
Copy link
Member

haslinghuis commented Aug 26, 2021

If (nightly) builds produces an APK there is little value to have this available through the store. Otherwise it's just a convenience to have debug versions available through the store beside release versions. So if it's technical possible there is some value for the lazy ones. 😛

@mikeller
Copy link
Member Author

@haslinghuis: I am not even sure there is any value in getting debug versions published through the store - the way the store works users have no other option that to use whatever the latest version to be released is, so if they want to debug something with a specific version that is not the latest they will have to uninstall the store version and install the .apk version.

asizon
asizon previously approved these changes Aug 26, 2021
@asizon asizon added the Tested label Aug 26, 2021
@McGiverGim
Copy link
Member

The Play Store have the option to suscribe to a "beta" of an app. If you are subscribed, you will receive "beta" versions of the app. If not, you receive only the stable version. You can opt in and out of this beta version when you want. My idea was to publish nightlies as this beta version, but as you say, this is a first step and can be done later if we can find it valuable.

@mikeller
Copy link
Member Author

@McGiverGim: We do not have a 'beta' channel of configurator - all we have are unstable development versions that may or may not work.

@mikeller
Copy link
Member Author

Blocking this to make sure #2574 is merged first.

@McGiverGim
Copy link
Member

@McGiverGim: We do not have a 'beta' channel of configurator - all we have are unstable development versions that may or may not work.

Beta is a way to say it... we can say it Alpha or whatever we want. Is simply a channel with development versions for those who want to help to find bugs. But as I said before, if nobody things it can be useful, I have no problem for it.

@haslinghuis
Copy link
Member

@mikeller removed the label as 4 commits are not auto-merged. Please just wait for the other PR to be merged and squash this one.

@mikeller
Copy link
Member Author

@haslinghuis: Good point - one of them is #2574, which means I need to wait until this one has been merged before doing the rebase.

@mikeller
Copy link
Member Author

@McGiverGim: I think for testing it is more helpful if we keep providing the .apk files that we are providing now - this allows the user to test with a specific version, or even with a build from a branch. If we publish the 'development' version in the Play store this will not be possible any more, unless the user first uninstalls the version from the store.

haslinghuis
haslinghuis previously approved these changes Aug 31, 2021
@mikeller mikeller force-pushed the add_play_store_build branch from 98f5fb5 to 9f0ccee Compare September 7, 2021 04:49
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 7, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.3% 0.3% Duplication

@blckmn
Copy link
Member

blckmn commented Sep 7, 2021

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> FAIL
  • assigned to a milestone -> PASS
  • cooling off period lapsed -> PASS
  • commit count less or equal to three -> PASS
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> PASS
  • Tested label found -> PASS
  • assigned to an approver -> PASS
  • approver count at least three -> FAIL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mobile support?

6 participants