Move Tideways API key from global config to per-project config#82
Merged
Conversation
The Tideways API key is per *Tideways project*, so it cannot live in ~/.magebox/config.yaml — a developer with two Magento projects belonging to two Tideways projects needs two different keys. The previous implementation wrote a single machine-wide api_key to /etc/php/8.x/mods-available/tideways.ini, which either silently pinned all local projects to the same Tideways project or got shadowed by whatever project was most-recently-configured. MageBox already has the right primitive: the .magebox.yaml `php_ini` map is rendered into each project's FPM pool config as `php_admin_value` (see internal/php/templates/pool.conf.tmpl:53-56), so per-project overrides of any PHP ini directive are a one-line yaml edit. The fix is to stop writing api_key globally and tell users to set it there. Changes: - Drop `api_key` from the global config schema. The `APIKey` field on TidewaysCredentials is kept (marked Deprecated) purely so legacy configs still unmarshal and can be detected via HasLegacyTidewaysAPIKey() for a migration warning. - Drop `HasTidewaysCredentials` and `TIDEWAYS_API_KEY` env var handling. Access token and environment remain as global settings. - Remove WriteAPIKeyToExtension and rewriteIniDirective from the tideways manager — no more writes to the global extension ini. In their place, add CleanLegacyExtensionDirectives which strips any `tideways.api_key` / `tideways.environment` lines that older MageBox versions wrote there. Nothing is written back except the cleaned file. - Remove APIKey from tideways.Credentials and the unused Configured bool + IsFullyConfigured method from tideways.Status. - `magebox tideways config` is now global-only: prompts for access token and environment, calls CleanLegacyExtensionDirectives for every installed PHP version, writes the systemd drop-in for the daemon environment, imports the CLI token. If a legacy api_key is found in ~/.magebox/config.yaml, it prints a migration warning and removes it on save. Reloads PHP-FPM only when the ini was actually modified. - `magebox tideways status` now reads the project's .magebox.yaml (via config.LoadFromPath) and reports whether php_ini.tideways.api_key is set for *this* project, with a yaml snippet in the warning when it isn't. Also surfaces the legacy-api-key warning. Tests: - global_test.go: drop TIDEWAYS_API_KEY env var cases, add TestGlobalConfig_HasLegacyTidewaysAPIKey, add TestGlobalConfig_TidewaysLegacyAPIKeyStillUnmarshals to verify old configs still round-trip so migration can detect them. Assert that GetTidewaysCredentials never surfaces APIKey. - manager_test.go: replace rewriteIniDirective cases with an expanded stripIniDirective suite (both directives, commented/uncommented, whitespace prefix, multiple occurrences) plus a composed-cleanup case that exercises the two-step strip flow in CleanLegacyExtensionDirectives. Keep the systemd drop-in test.
When 'magebox tideways config' is run from inside a project that has no tideways.api_key set in its merged php_ini map, prompt for the key and write it to .magebox.local.yaml under php_ini.tideways.api_key. .magebox.local.yaml is the right home: it is the personal override file and typically gitignored, so the api_key (which is a secret) stays out of the shared repo. Users who want to share the key with their team can move it to .magebox.yaml manually afterward. - New --project-api-key flag for non-interactive use (only applied when MageBox is actually inside a project). - Interactive flow: if cwd is a project without the key already set, print the project name and ask for the key after the Environment prompt. Leaving it blank is a no-op — the command still prints a reminder with a yaml snippet. - writeProjectTidewaysAPIKey uses LoadLocalConfig / SaveLocalConfig so existing php_ini entries (memory_limit, xdebug.mode, etc.) and the env section are preserved. Allocates the PHPINI map first if nil. - After writing, print a reminder to run 'magebox restart' so the FPM pool picks up the new php_admin_value. - Updated the config command's Long description to document both the global and project scopes, and dropped the now-stale final hint (\"Set the API key per project ...\") when we're actually inside a project — it's redundant with the prompt. Tests: three cases covering creating a new .magebox.local.yaml, merging into an existing one without clobbering other php_ini/env entries, and handling an existing file with no php_ini section (nil map).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Tideways API key is per Tideways project, so it cannot live in
~/.magebox/config.yaml— a developer with two Magento projects belonging to two Tideways projects needs two different keys. The previous implementation (merged as #77, released in v1.14.1) wrote a single machine-widetideways.api_keyto/etc/php/8.x/mods-available/tideways.ini, which either silently pinned all local projects to the same Tideways project or got shadowed by whatever project was most-recently-configured.MageBox already has the right primitive: the
.magebox.yamlphp_inimap is rendered into each project's FPM pool config asphp_admin_value(seeinternal/php/templates/pool.conf.tmpl:53-56), so per-project overrides of any PHP ini directive are a one-line YAML edit:```yaml
php_ini:
tideways.api_key: your-project-specific-key
```
That's the right home for the API key — no new schema, no new command, it just works. This PR stops writing the key globally and surfaces the per-project location everywhere a user would look.
Changes
api_keyis no longer prompted, stored, or written. Kept as a// Deprecated:field on `TidewaysCredentials` purely so legacy configs still unmarshal and can be detected via `HasLegacyTidewaysAPIKey()` for migration. `HasTidewaysCredentials` and `TIDEWAYS_API_KEY` env var handling removed.Tests
All existing tests pass; lint clean.
Test plan