Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

API Keys read from environment variables #1075

Merged
merged 12 commits into from Nov 25, 2017
Merged

API Keys read from environment variables #1075

merged 12 commits into from Nov 25, 2017

Conversation

Sherlouk
Copy link
Member

No description provided.

let processInfo = ProcessInfo.processInfo

guard let value = processInfo.environment[named] else {
print("‼️ Missing Environment Variable: '\(named)'")
Copy link
Member Author

Choose a reason for hiding this comment

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

We could do #if DEBUG then fatalError() if we wanted to be super obvious when doing things locally?

@Sherlouk
Copy link
Member Author

Not sure who has the most recent CP, probably Ryan, feel free to regen CP to revert those changes - did it to ensure everything was coupled correctly!

@Sherlouk
Copy link
Member Author

Sherlouk commented Nov 23, 2017

Other notes:

Commented out FBSnapshot tests as they're failing on CI, I attempted to change a few things but it still didn't work 😞
There was issues with linking files, so I did the change from #1015 as part of this and that's now fixed.
Was a couple issues with the recent Accessibility stuff, also fixed that (just needed an import)
Added tests for Secrets to absolutely verify that if it passes on BB - it should be fine.

@rnystrom
Copy link
Member

So what’s the process for adding your keys as env vars?

Sent with GitHawk

@Sherlouk
Copy link
Member Author

Let me get it working first then we can work that bit out 😂

@Sherlouk
Copy link
Member Author

Haven't tested it, but according to https://docs.fastlane.tools/best-practices/keys/ we can define environment variables via ~/.bash_profile and they should apply to the builds

GITHUB_CLIENT_ID
GITHUB_CLIENT_SECRET
IMGUR_CLIENT_ID

Next build from BB should pass 🤞 I've run all the tests I wanted to so I'm happy the value is getting passed all the way through. Just need to validate the fastlane route but that's more for you @rnystrom

If devs want to device the keys above, they should also work for local builds via simulator. Or you can define them via Edit Schemes -> Run -> Environment Variables but that'll only work while on debugger!

@Sherlouk Sherlouk changed the title WIP: Fix Master Attempt 4 API Keys read from environment variables Nov 23, 2017
@Sherlouk
Copy link
Member Author

Just installed the build from BB, can confirm all the keys are there and working fine. So if we deploy using that were solid!

@rnystrom
Copy link
Member

Awesome. Would love to start deploying via BB once the build number works (wasn’t getting full commit count last time I tried).

So to ship locally via FastLane I’ll need to modify my bash profile? Will anything fail so we can’t ship without the keys set?

Sent with GitHawk

@Sherlouk
Copy link
Member Author

Sherlouk commented Nov 24, 2017

@rnystrom So currently I have a check if [ -z "${BUDDYBUILD_BUILD_NUMBER}" ]; then which basically says to validate that the env variables exist. This is what will make BB builds fail if it doesn't have the vars (at least it should, I didn't test it on BB but locally it was picking things up perfectly).

I've added the check so that you can continue locally without having to define them.

If we were to put ENV["BUDDYBUILD_BUILD_NUMBER"] = "1" at the top of the fastlane it would be enough to trick it into running the check (and thus should fail). Maybe don't add the github env vars, put add the BB build number to the fastfile as above and see if it fails for you? If it fails then great, define the vars and see it go through otherwise we can re-approach it.

I would test it, but I don't have the profiles to try the build phase!

@Sherlouk
Copy link
Member Author

As for BB, it looks fine from my UI and we don't even need to use the commit counter. You can give it an abritrary number and it'll just increment each release (which would be fine for us) or just use the "timestamp" option which will always be unique!

@rnystrom
Copy link
Member

Time stamp sounds great 👌

Sent with GitHawk

@rnystrom
Copy link
Member

I haven’t checked this out locally, but what are the setup steps required to use keys on debug builds? Should we update the Contributing guide in this PR too?

Planning on checking this out today and giving feedback on setup/usage.

Sent with GitHawk

@Sherlouk
Copy link
Member Author

Sherlouk commented Nov 24, 2017

Okay so there are multiple cases here:

BuddyBuild

  • I've added "Environment Variables" to the BuddyBuild dashboard for the three necessary values. This is confirmed to be working.

FastLane

  • You will need to define your own environment variables as per this guide around the .bash_profile stuff
  • If you want your build to fail if the keys can't be found then you will also need to define a BUDDYBUILD_BUILD_NUMBER. We could change this to be something else.
  • Using the same guide as above, if we add ENV["BUDDYBUILD_BUILD_NUMBER"] = "1" to the top of the fastfile this will be enabled

Local Debug Builds

  • If you want to quickly enable these keys then you go to Edit Scheme -> Run -> Arguments -> Click the '+' and add a new Key/Value pair for each of these variables: GITHUB_CLIENT_ID, GITHUB_CLIENT_SECRET and IMGUR_CLIENT_ID. (Confirmed working)
  • This will not work if you remove the debugger and open the app yourself as the variables are then not found.
  • If you want to always have these variables defined, you can follow the first step of the Fastlane case in saving the environment variables to your .base_profile - this should also work locally.

Reminder: I've added the Secrets.swift file to the .gitignore - do not forcefully push changes to this file unless you are trying to alter the script. In the case of defining environment variables outside of the scheme, this file is overridden to insert the API keys directly in the code (this is how I have gotten it working for release builds)

Feel free to write this up bette and update the contributing guide!

image

@rnystrom
Copy link
Member

rnystrom commented Nov 24, 2017

@Sherlouk ok so I tried exporting my keys via

export GITHUB_CLIENT_ID='123'
export GITHUB_CLIENT_SECRET='abc'
export IMGUR_CLIENT_ID='456'

And nothing passes the unit test:

  • ~/.zshrc (I use zsh)
  • ~/.bash_profile like fastlane uses
  • ~/.bashrc

How do you get this to work locally in debug builds w/out editing checked-in project files?

edit:

Ok great, adding the keys to the env variables for the scheme works and doesn't change any project files b/c they are stored in Freetime.xcodeproj/xcuserdata/rnystrom.xcuserdatad/xcschemes/Freetime.xcscheme which is ignored.

@Sherlouk
Copy link
Member Author

@rnystrom So unit tests won't pass unless you've enabled the job, which as per the previous comment requires BUDDYBUILD_BUILD_NUMBER to also be defined (assuming you're doing the env variable route as you describe; if you do the scheme route for local builds then it isn't necessary)

@rnystrom rnystrom merged commit 0dffae0 into master Nov 25, 2017
@rnystrom rnystrom deleted the fix-builds branch November 25, 2017 01:51
@Sherlouk
Copy link
Member Author

🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants