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

Error whilst using docker for for the FileChanged/TypeOf condition #211

Closed
cegento opened this issue Apr 13, 2023 · 14 comments
Closed

Error whilst using docker for for the FileChanged/TypeOf condition #211

cegento opened this issue Apr 13, 2023 · 14 comments
Assignees

Comments

@cegento
Copy link

cegento commented Apr 13, 2023

When using the following config

"config": {
 "run-mode": "docker",
 "run-exec": "docker exec docker_container_1"
}

into the captainhook.json. This causes the condition to always return false and therefore the action is skipped.
I dont know how exactly the software works, but when i looked into the code, it seemed like, that no branchdata + commit was send to to getStandardInput() in class \CaptainHook\App\Console\IO\DefaultIo.
In comparison to a previous test (where i removed the config part out of captainhook.json) the data could be retrieved from that class and therefore it also worked correct.

@sebastianfeldmann
Copy link
Collaborator

Can you show the configuration part of the Condition and tell me the git operation you are running.
Currently it's a bit vague in order to make any conclusions.

Please provide:

  • complete hook or at least action config
  • executed git command

Be sure that your container can execute git and has access to the .git directory.

@sebastianfeldmann sebastianfeldmann self-assigned this Apr 13, 2023
@cegento
Copy link
Author

cegento commented Apr 13, 2023

complete hook or at least action config

The part of the configuration:

{
"config": {
 "run-mode": "docker",
 "run-exec": "docker exec docker_container_1"
},
"pre-push": {
        "enabled": true,
        "actions": [
            {
                "action": "composer run test",
                "options": [],
                "conditions": [
                    {
                        "exec": "or",
                        "args": [
                            {
                                "exec": "\\CaptainHook\\App\\Hook\\Condition\\FileChanged\\OfType",
                                "args": [
                                    "php"
                                ]
                            },
                            {
                                "exec": "\\CaptainHook\\App\\Hook\\Condition\\FileChanged\\OfType",
                                "args": [
                                    "phtml"
                                ]
                            }
                        ]
                    }
                ]
            }
        ]
    }
 }

executed git command

The command i ran:
git push

Be sure that your container can execute git and has access to the .git directory.

Yes it has access to .git and it can execute git push correctly (if hooks disabled)

@sebastianfeldmann
Copy link
Collaborator

The question is, where do you execute git push?
Within the container, or one the host system?

If you run git commands on the host system and execute the Cap'n inside a container you have to make sure that the Cap'n inside that container has access to a git executable and has access to the .git directory.
The Cap'n executes git commands in the background to make decisions.

There is another problem however.
You can not use the StagedFiles conditions in pre-push hooks.
Staging (add to index) only works in pre-commit hooks.

    /**
     * Return the FileStaged\OfType restriction information
     *
     * @return \CaptainHook\App\Hook\Restriction
     */
    public static function getRestriction(): Restriction
    {
        return Restriction::fromArray([Hooks::PRE_COMMIT]);
    }

You need to use FileChanged\OfType because that is applicable for pre-push hooks.

   /**
     * Return the FileChanged\OfType restriction information
     *
     * @return \CaptainHook\App\Hook\Restriction
     */
    public static function getRestriction(): Restriction
    {
        return Restriction::fromArray([Hooks::PRE_PUSH, Hooks::POST_CHECKOUT, Hooks::POST_MERGE, Hooks::POST_REWRITE]);
    }

Maybe I should echo a warning if a condition is wrongly used.

But as mentioned before, you have to make sure that the container executing CaptainHook has access to git and the .git directory.

@cegento
Copy link
Author

cegento commented Apr 17, 2023

The question is, where do you execute git push?

I run it on the host system (outside of docker)

You can not use the StagedFiles conditions in pre-push hooks.

My bad, i actually ment \CaptainHook\App\Hook\Condition\FileChanged\OfType (at least this was what i used when testing it). Copy past error since i was in the wrong branch.

If you run git commands on the host system and execute the Cap'n inside a container you have to make sure that the Cap'n inside that container has access to a git executable and has access to the .git directory.

Testet it by putting echo shell_exec("cat .git/HEAD"); into the captainhook code. It correctly outputted the file data

@sebastianfeldmann
Copy link
Collaborator

Another test you have to do is to use git status as an action and see if that runs.
Only if the Cap'n has access to git it can execute properly.

@cegento
Copy link
Author

cegento commented Apr 23, 2023

Another test you have to do is to use git status as an action and see if that runs.

Tried it by putting echo shell_exec("git status") into the code and it worked

@sebastianfeldmann
Copy link
Collaborator

Have you tried to run the Cap'n in debug mode?

captainhook -vvv

or in your configuration

  "config": {
    "verbosity": "debug"
  }

@cegento
Copy link
Author

cegento commented Apr 27, 2023

I have now added "verbosity": "debug" to the config part. I must state that the output didn't really change that much except that messages like No plugins to execute for: beforeHook popped up.

@sebastianfeldmann
Copy link
Collaborator

Can you post the full output? Of course you can remove all sensitive data like path or command options or even commands.

Just to recap. You are running git push and would expect the conditions to be true and want composer run test to be executed. Right?

@cegento
Copy link
Author

cegento commented May 1, 2023

Can you post the full output? Of course you can remove all sensitive data like path or command options or even commands.

user@user:/var/www/project$ git push
pre-push: 
No plugins to execute for: beforeHook
 - composer run test                                                :  skipped
 - bin/magento module:status --enabled | grep -e App_ -e Stat... : 
No plugins to execute for: beforeAction

[From now on it just gets pushed regularely, therefore only git log]

Just to recap. You are running git push and would expect the conditions to be true and want composer run test to be executed. Right?

Exactly. For more context, i have committed a file with a .php ending to the local repository already and therefore i would expect composer run test to be executed inside the docker container (but as you see, it gets skipped).

@cegento
Copy link
Author

cegento commented May 29, 2023

I have decided to make a detailed step by step list so you can reproduce it too.

  1. Create a folder called test
  2. Go in it and run git init and upload it on github
  3. Install composer on the hostmachine + php 8.1.2
  4. Install docker and docker-compose if you dont have it
  5. Create a docker-compse.yml file and put the following in it:
services:
  php-fpm:
    image: 'bitnami/php-fpm'
    volumes:
      - ./:/app
  1. run docker-compose up
  2. Install captainhook with composer require captainhook/captainhook
  3. Add a captainhook.json with the following content. Note that test_php-fpm_1 might have a different name on your machine. If you dont know the name, type docker container list and replace it with the name of the docker container of php-fpm.
{
    "config": {
        "run-mode": "docker",
        "run-exec": "docker exec test_php-fpm_1" 
    },
    "pre-push": {
        "enabled": true,
        "actions": [
            {
                "action": "echo \"test\"",
                "options": [],
                "conditions": [
                    {
                        "exec": "or",
                        "args": [
                            {
                                "exec": "\\CaptainHook\\App\\Hook\\Condition\\FileChanged\\OfType",
                                "args": [
                                    "php"
                                ]
                            },
                            { 
                                "exec": "\\CaptainHook\\App\\Hook\\Condition\\FileChanged\\OfType",
                                "args": [
                                    "phtml"
                                ]
                            }
                        ]
                    }
                ]
            },
            {
                "action": "exit",
                "options": []
            }
        ]
    }
}

  1. Then run vendor/bin/captainhook install in the git repository path
  2. Add a .gitignore file and put vendor in it.
  3. Push everything so that you have a clean git enviroment
  4. Add a .php file that is called lets say "test.php".
  5. run git add .
  6. run git commit -m "Test"
  7. run git push
  8. From now on you will see that the pre-push action echo "test" will be skipped, even though, there is a new php file.

@sebastianfeldmann
Copy link
Collaborator

So if I do everything as you described I can reproduce your error.

If I add the following to the configuration

"pre-push": {
        "enabled": true,
        "actions": [
            {
                "action": "git status"
            },
            {
                "action": "echo \"TEST TEST\"",
                "options": [],
                "conditions": [
                    {
                        "exec": "or",
                        "args": [
                            {
                                "exec": "\\CaptainHook\\App\\Hook\\Condition\\FileChanged\\OfType",
                                "args": [
                                    "php"
                                ]
                            },
                            {
                                "exec": "\\CaptainHook\\App\\Hook\\Condition\\FileChanged\\OfType",
                                "args": [
                                    "phtml"
                                ]
                            }
                        ]
                    }
                ]
            },
            {
                "action": "\\CaptainHook\\App\\Hook\\Debug"
            }
        ]
    }

I get the following error.

failed to execute: git status

sh: 1: git: not found

That means the container does not have access to git. Without access to git the Captain can not figure out what files have changed. I'm currently investigating why the Condition is not failing, because without access to git everything should crash and scream "NO GIT". I will investigate and hopefully find the problem why it doesn't.

@sebastianfeldmann
Copy link
Collaborator

sebastianfeldmann commented Jun 1, 2023

Ok the problem is the docker exec you are using.

The start and end point of pushes is communicated via stdin to the hook.
If you delegate the hook execution to a container you have to forward the stdin by using the -i option.

    "run-exec": "docker exec -i dummy-php-fpm-1"

That worked in my case. At least now the Cap'n sees the start and end point of the push and tries to figure out what changed. If the info is missing the Cap'n expects a basically empty push so he isn't doing anything, That's why the Condition does not trigger.

It works to the point were the Cap'n tries to figure out what changed. Because for that the Cap'n uses some git commands and without access to git it does not work and crashes.

But as soon you pass the stdin to your container and the container has access to git it should work as expected.

@cegento
Copy link
Author

cegento commented Jun 4, 2023

Thank you for helping!, it now works for me too

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

No branches or pull requests

2 participants