dokku / ansible-dokku

Ansible modules for installing and configuring Dokku
MIT License
156 stars 44 forks source link

Update dokku-daemon.yml #184

Closed robotski closed 4 months ago

robotski commented 4 months ago

GitHub's SSH key changed

https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/githubs-ssh-key-fingerprints

Should possibly add the Ed25519 key (and maybe remove the RSA one?) but those seem like more significant changes.

ltalirz commented 4 months ago

Thanks a lot @robotski

robotski commented 4 months ago

@ltalirz I've done a bit of debugging on my end, and it seems like the CI failure is being caused by a bug in Dokku.

If I run dokku apps:create example-app followed by dokku git:sync example-app https://github.com/heroku/node-js-getting-started b10a4d7a20a6bbe49655769c526a2b424e0e5d0b --build, then the the git sha according to dokku git:report is b10a4d7. If I then run git:sync again without the --build, the sha becomes b10a4d7a and doesn't change again from any combination of the two git:sync commands.

I've opened up an issue in the main repo: dokku/dokku#6770

ltalirz commented 4 months ago

hi @robotski , thanks for looking into this!

Reading through your comment, it seems to me that the git sha reported is actually correct in all cases?

git has some logic to determine what fraction of the sha to display, i.e. it's possible that it decides after some operation that it now needs to display a longer fraction of the sha in order for the displayed fraction to be unique.

if some code relies on these "short versions" of shas to be identical, then probably that is the bug (one should compare full shas instead)

mentioning @josegonzalez for info

robotski commented 4 months ago

I wonder if it might be appropriate to add a --git-sha-long option to dokku git:report then? It doesn't make sense for this Ansible role to do manual Git operations, IMO.

The currently existing --git-sha option being called by module_utils/dokku_git.py (used by library/dokku_clone.py to figure out if the app has changed) effectively runs git "$APP_ROOT" rev-parse --short HEAD. The --short option by itself returns the shortest unique sha, but using --short=10 or --short=12 would prevent the inconsistency AND hash collisions for any repo smaller than the Linux kernel.

Or the sha comparison in dokku_clone.py could truncate the longer hash, but that seems like an incorrect solution to me.

ltalirz commented 4 months ago

I wonder if it might be appropriate to add a --git-sha-long option to dokku git:report then? It doesn't make sense for this Ansible role to do manual Git operations, IMO.

Yes. Happy to review a PR

josegonzalez commented 4 months ago

The dokku side should probably just always use a long commit-sha. Don't know if I feel this is a breaking change or not though.

robotski commented 4 months ago

@ltalirz Could you please re-run CI when you get a chance? There's a new release of Dokku out that fixes the sha issue. Thanks!

ltalirz commented 4 months ago

In one instance the test on ubuntu2204 failed with

  TASK [dokku_bot.ansible_dokku : clone dokku-daemon] ****************************
  fatal: [instance]: FAILED! => {"changed": false, "cmd": "/usr/bin/git ls-remote origin -h refs/heads/0.0.2", "msg": "fatal: unable to access 'https://github.com/dokku/dokku-daemon.git/': GnuTLS recv error (-54): Error in the pull function.", "rc": 128, "stderr": "fatal: unable to access 'https://github.com/dokku/dokku-daemon.git/': GnuTLS recv error (-54): Error in the pull function.\n", "stderr_lines": ["fatal: unable to access 'https://github.com/dokku/dokku-daemon.git/': GnuTLS recv error (-54): Error in the pull function."], "stdout": "", "stdout_lines": []}

Not sure where this comes from? In any case I'll merge for now

ltalirz commented 4 months ago

Apparently this has to do with http post buffer sizes

robotski commented 3 months ago

Would it be possible to publish to Galaxy soon? The current release is unable to clone repos.

ltalirz commented 3 months ago

just pushed the tag, let me know whether it works (will take 30 minutes to publish to galaxy)