Fix Secrets setup for external contributors

[AliSoftware]

Why?

In a recent Slack conversation (p1630395082048000/1630357386.017300-slack-C02Q9GACF) with @autoblork, we realized that the rake init:oss task – which is used by external contributors and trialees as instructed in the README, and ends up asking them their clientID and Secret to generate the Secrets.swift file for them – generated the file in the old location we used before recently moving the Secrets (.configure-files).

How?

• This PR updates the path in the Rakefile to match the path used in Scripts/BuildScripts/ApplyConfiguration.sh's $LOCAL_SECRETS_FILE, aka "${SRCROOT}/Credentials/Secrets.swift".

  • That way when the script build phase is run and tries the various locations to find the Secrets source to copy over and fails to find Production/Internal/Alpha/Jetpack and falls back to $LOCAL_SECRETS_FILE, it finds the expected file generated for/by external contributors during rake init:oss there.

• That WordPress/Credentials/Secrets.swift file has also been added to the .gitignore file so that external contributors don't accidentally commit it to their fork.

• Finally, it moves the secrets-manifest.xcfilelist inside the WordPress/Credentials/ folder (instead of being at the $SRC_ROOT) to tidy things up as a bonus.

Local Secrets file: in vs out of working copy

Given the goal of the recent move of our Secrets outside <repo>/.configure-files and into $HOME was made in order to have a stronger security at avoiding the risk of accidentally comitting production secrets and credentials to GitHub, I wondered about having this local secrets file here vs in the user's $HOME.

My rationale for keeping that file here though is that, it being for external contributors (and not for a12s who might use production secrets), it makes it easier for those external (and potentially just occasional) contributors to find/edit the file if needed, and also less invasive for those external contributors to have this file in their working copy than, say, have us "pollute" their $HOME folder.

Personally, I'd hate as an external contributor to an OSS project if that project would start creating files in various places in my computer outside its working directory, so I feel that this choice is more respectful and proper etiquette in that context.

\cc @mokagio do you agree ☝️ ?

[peril-wordpress-mobile]

You can trigger an installable build for these changes by visiting CircleCI here.

[peril-wordpress-mobile]

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

[ghost]

Tried a clean clone of this branch and it solves the issue for me 👍

[mokagio]

Tested locally and it worked like a charm 👍

it makes it easier for those external (and potentially just occasional) contributors to find/edit the file if needed, and also less invasive for those external contributors to have this file in their working copy than, say, have us "pollute" their $HOME folder.

+1

I was expecting a warning in the log about the local secrets being used. My expectation was wrong, though, because the script never output one. Still, I think it would be useful to print one as a reminder. What do you think?

I can work on it later on. I'd like to also update the warning for missing secrets on non-Release builds to suggest running rake init:oss so I might batch them.

[AliSoftware]

@mokagio I pushed the following updates:

• Updated the script to implement your suggestions (warning message on use of local secret, update error message to suggest rake init:oss)
• Moved the xcfilelist next to the script
• Moved the script to an Aggregate Target that I made a dependency of WordPress and Jetpack targets, instead of being a Build Phase.
  • This reduces significantly the time it takes before getting an error about missing Secrets or an external contributor trying to do a Release build 🎉 by running the script in parallel without having to wait for Pods to finish building before we get to it.
  • Renamed the script and the xcfilelist according to the Aggregate Target's name (GenerateCrendentials) for consistency
• Made the script always run, even in incremental builds, so that we always get the proper warning/error in all cases.

To test this, I temporarily renamed ~/.configure to ~/.configure.bak in my $HOME, then:

• Built with Debug configuration:
  
• Built with Release-Internal configuration:
  

Then, I duplicated Secrets-example.swift into Secrets.swift next to it (to simulate init:oss having created that file) and:

• Built with Debug configuration:
  
• Built with Release-Internal configuration:
  

---

@mokagio I'll let you test that this works as expected in all cases for you too, and have re-requested a review from you to be sure you see and check those new changes.

[mokagio]

Thanks @AliSoftware. I'm going to bump this to 18.3 so I can focus on the 18.2 code freeze first. The changes in this setup are not user facing, so merging them before or after the code freeze makes little difference. (Not that you expressed a particular desire to have it in 18.2 😅, but just to explain my rationale for bumping this)

[AliSoftware]

Doing a dummy comment in the hopes to re-trigger Peril and get it go green, as we fixed the issue with Hold context on Jetpack in Automattic/peril-settings#90.

[EDIT] Ok, neither comment nor label dance was enough to re-trigger Peril, which makes sense in retrospect, because this one is only listening to Webhook events about status changes obviously… so ended up triggering the Jetpack Installable Build anyway to make it go out of its amber status 🙃

[mokagio]

Took it for a spin locally after deleting ~/.configure, worked as described.

Thank you for also going the extra mile and moving the script into the aggregated target. That's super cool and definitely a welcome and long due improvement 👏