Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use vip dev-env configuration file from parent directories #1468

Merged
merged 23 commits into from
Jan 22, 2024

Conversation

alecgeatches
Copy link
Contributor

@alecgeatches alecgeatches commented Jul 25, 2023

Description

Extends configuration file support to work in any subdirectory of a VIP site.

vip dev-env commands run anywhere within a configured site will automatically use the site's configuration file. vip dev-env create will pull environment settings from the .wpvip configuration folder, and subsequent vip dev-env commands will use the site's configured slug. There's no longer any need to remember a slug or where configuration is setup as long as it's in the root of a VIP site.

In the demo below, a skeleton site is set up with this structure:

vip-go-skeleton/
├── .wpvip/
│   └── vip-dev-env.yml      <-- Configuration file here
├── client-mu-plugins/
├── ...
├── themes/
│   └── twentytwentythree/   <-- CLI commands run here
├── private/
└── vip-config/

The video demonstrates using the same site's development environment in the site root, and within a theme folder:

vip-dev-env-version-1.mp4

Additional changes

  1. The .vip-dev-env.yml file previously could be located anywhere. For consistency, the file is now always loaded from the configuration directory in .wpvip/vip-dev-env.yml. Because the configuration directory is hidden, the . has been removed from the vip-dev-env.yml filename.
  2. Changed the supported configuration-version from 0.preview-unstable to 1.
  3. Removed echoing the whole configuration file on vip dev-env create.

Steps to Test

Manually

These changes can be tested globally via npm run build && npm link using the add/configuration-file-from-ancestor branch, or commands can be run directly from compiled files like this:

$ cd vip-cli
$ npm run build
$ node dist/bin/vip-dev-env-start.js
  1. Clone vip-go-skeleton into a new directory:

    $ git clone [email protected]:Automattic/vip-go-skeleton.git
  2. In the vip-go-skeleton/ root directory, create a .wpvip/ folder containing a vip-dev-env.yml configuration file:

    $ cd vip-go-skeleton
    $ mkdir .wpvip
    $ vim .wpvip/vip-dev-env.yml
  3. Add the following configuration file contents:

    configuration-version: 1
    slug: test-ancestor-configuration
    title: Test ancestor configuration
    php: 8.0
    wordpress: 6.2
    app-code: ../
    mu-plugins: image
    xdebug: false
    mailpit: false
    multisite: false
    elasticsearch: false
    phpmyadmin: false
    photon: false
  4. In the same root vip-go-skeleton/ directory, create and run the environment:

    $ vip dev-env create
    $ vip dev-env start
    Using environment test-ancestor-configuration from /Users/alec/.../vip-go-skeleton/.wpvip/vip-dev-env.yml
    
    # ...
    SLUG              test-ancestor-configuration                                                                                          
    LOCATION          /Users/alec/.local/share/vip/dev-environment/test-ancestor-configuration                                             
    # ...                                                                                                     

    vip dev-env create/vip dev-env start commands can also be run in any subdirectory of the configured skeleton site.

  5. Verify that vip dev-env commands work in subdirectories of vip-go-skeleton/

    $ cd plugins/
    $ vip dev-env info
    Using environment test-ancestor-configuration from /Users/alec/.../vip-go-skeleton/.wpvip/vip-dev-env.yml
    # ...
    SLUG              test-ancestor-configuration                                                                                          
    LOCATION          /Users/alec/.local/share/vip/dev-environment/test-ancestor-configuration                                             
    # ...  
    
    $ vip dev-env exec -- wp option get siteurl
    Using environment test-ancestor-configuration from /Users/alec/.../vip-go-skeleton/.wpvip/vip-dev-env.yml
    
    Running validation steps...
    # ...
    http://test-ancestor-configuration.vipdev.lndo.site
  6. Try entering deeper subdirectories (e.g. something in themes/) and ensure vip dev-env commands automatically pull the environment slug from the root site directory.

Tests

New end-to-end tests have been added in __tests/devenv-e2e/013-configuration-file.spec.js. Run these tests with:

$ npm run test:e2e:dev-env 013

@alecgeatches alecgeatches self-assigned this Jul 31, 2023
@@ -74,9 +74,14 @@ export interface ConfigurationFileOptions {
'media-redirect-domain'?: string;
photon?: boolean;

meta?: ConfigurationFileMeta;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this meta key for ConfigurationFileOptions to store information about the configuration file that doesn't affect the instance. This meta currently stores configuration-path which is used to show this configuration status line:

configuration-file-meta

@alecgeatches alecgeatches marked this pull request as ready for review August 2, 2023 21:54
@alecgeatches
Copy link
Contributor Author

alecgeatches commented Aug 2, 2023

Note: I see there are some type issues causing test failures, fixing those now.

@rinatkhaziev
Copy link
Contributor

Just a note on mailhog vs mailpit, since we ditched the former probably need to update all instances. Step 3: mentions mailhog

@alecgeatches
Copy link
Contributor Author

Just a note on mailhog vs mailpit, since we ditched the former probably need to update all instances. Step 3: mentions mailhog

Thank you! The code appears to be updated to work with mailpit/fallback to mailhog properly. Updated testing instructions to use mailpit instead.

@alecgeatches
Copy link
Contributor Author

The tests have been fixed and this is ready for review.

Copy link
Contributor

github-actions bot commented Jan 7, 2024

This pull request has been marked stale because it has been open for 60 days with no activity. If there is no activity within 7 days, it will be closed.

This is an automation to keep pull requests manageable and actionable and is not a comment on the quality of this pull request nor on the work done so far. Closed PRs are still valuable to the project and their branches are preserved.

@alecgeatches
Copy link
Contributor Author

Commenting to unstale this. I'm going to re-merge trunk and get this reviewed.

@rinatkhaziev
Copy link
Contributor

@alecgeatches I took the liberty and merged the trunk.

Copy link
Contributor

@rinatkhaziev rinatkhaziev left a comment

Choose a reason for hiding this comment

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

Tested, works great. Even went as far as:

mkdir -p 1/2/3/4/5/6/7/8/9/10
cd 1/2/3/4/5/6/7/8/9/10 

vip dev-env start

On the other hand, this also works:

 mv .wpvip ../
 /tmp/vip-go-skeleton   production  vip dev-env start
Using environment test-ancestor-configuration from /private/tmp/.wpvip/vip-dev-env.yml

I'm not sure whether this is an imaginary or a real problem, but this may lead to unintended consequences.

@yolih yolih added the [Status] Needs Docs The feature or update requires an update to our public VIP Documentation label Jan 10, 2024
Copy link
Contributor

@rinatkhaziev rinatkhaziev left a comment

Choose a reason for hiding this comment

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

After further discussion it seems that many other developer tools follow the same traversal strategy, so let's go with it.

We print the path to the file we use, so it's going to be easier to spot in case the unintended file is being used to start the env.

@mjangda
Copy link
Member

mjangda commented Jan 12, 2024

This is awesome.

Let's make sure to test on Windows as well so that we don't have any unexpected surprises there :)

In a future PR, it could be interesting to support a reference to a VIP environment in the config file for pulling defaults (similar to how you can do vip @123.develop dev-env create).

@rinatkhaziev
Copy link
Contributor

oh good point re: Windows. Booting up :)

@rinatkhaziev
Copy link
Contributor

Tested both under WSL and native Win, works as expected.

@alecgeatches alecgeatches force-pushed the add/configuration-file-from-ancestor branch from 009c260 to 631f443 Compare January 22, 2024 18:34
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@alecgeatches
Copy link
Contributor Author

Note: I made a final change in here to address the security fix made by @sjinks in #1652 in this commit: 631f443.

The new code moves readFile() into findConfigurationFile(), which returns both the configuration path and contents in a { configurationPath, configurationContents } object on success. This should address the same security issue by using a single readFile() call to check access and return file contents.

These changes also drop the ENOENT/error code checking, as we'll silently drop any errors and continue if a configuration file isn't available.

@alecgeatches alecgeatches merged commit e4bee14 into trunk Jan 22, 2024
16 checks passed
@alecgeatches alecgeatches deleted the add/configuration-file-from-ancestor branch January 22, 2024 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Needs Docs The feature or update requires an update to our public VIP Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants