fix: breaking GetRecipeVersions when an invalid recipe versions exist #750

2026-01-11T18:06:25Z

Apfelwurm commented

2026-01-11 18:06:25 +00:00

Currently, for example on the zammad recipe, we have a problem when using abra app new and abra app deploy where an old version, in zammads case 1.0.0+6.3.1-95 is selected and deployed.
This is due to invalid compose files in the tags 1.0.1+6.3.1-95, 1.0.2+6.3.1-95 and 1.0.3+6.3.1-95 which break GetRecipeVersions() to early, which prevents the detection of newer releases.

This change changes the behavior from erroring silently to warning the user about it and still iterating over the newer versions.

This results in this case for example in an output like this:

abra app new zammad            ░▒▓ ✔  4s    18:59:07   
WARN zammad: ignoring unsupported options: restart
WARN skipping tag 1.0.1+6.3.1-95: invalid compose config: yaml: unmarshal errors:
  line 104: mapping key "deploy" already defined at line 91
WARN skipping tag 1.0.2+6.3.1-95: invalid compose config: yaml: unmarshal errors:
  line 103: mapping key "deploy" already defined at line 90
WARN skipping tag 1.0.3+6.3.1-95: invalid compose config: yaml: unmarshal errors:
  line 103: mapping key "deploy" already defined at line 90

This also changes abra app deploy from directly using app.Recipe.Tags() to app.Recipe.GetRecipeVersions() so it is utilizing the same logic here.
This should be reviewed carefully, because i might not undestand all implications here.

Currently, for example on the zammad recipe, we have a problem when using `abra app new` and `abra app deploy` where an old version, in zammads case `1.0.0+6.3.1-95` is selected and deployed. This is due to invalid compose files in the tags 1.0.1+6.3.1-95, 1.0.2+6.3.1-95 and 1.0.3+6.3.1-95 which break GetRecipeVersions() to early, which prevents the detection of newer releases. This change changes the behavior from erroring silently to warning the user about it and still iterating over the newer versions. This results in this case for example in an output like this: ``` abra app new zammad ░▒▓ ✔ 4s  18:59:07  WARN zammad: ignoring unsupported options: restart WARN skipping tag 1.0.1+6.3.1-95: invalid compose config: yaml: unmarshal errors: line 104: mapping key "deploy" already defined at line 91 WARN skipping tag 1.0.2+6.3.1-95: invalid compose config: yaml: unmarshal errors: line 103: mapping key "deploy" already defined at line 90 WARN skipping tag 1.0.3+6.3.1-95: invalid compose config: yaml: unmarshal errors: line 103: mapping key "deploy" already defined at line 90 ``` This also changes abra app deploy from directly using `app.Recipe.Tags()` to `app.Recipe.GetRecipeVersions()` so it is utilizing the same logic here. This should be reviewed carefully, because i might not undestand all implications here.

🎉 1

Apfelwurm added 1 commit 2026-01-11 18:06:26 +00:00

fix: breaking GetRecipeVersions when an invalid recipe version exists

continuous-integration/drone/push Build is failing

Details

continuous-integration/drone/pr Build is failing

Details

ae9e49e1b5

Apfelwurm added the bug invalid labels 2026-01-11 18:11:06 +00:00

Apfelwurm marked the pull request as work in progress 2026-01-11 18:14:51 +00:00

ammaratef45 added 1 commit 2026-01-11 18:19:16 +00:00

Merge branch 'main' into fixBrokenRecipeCheckout

continuous-integration/drone/push Build is failing

Details

683a3bbf3d

Apfelwurm added 1 commit 2026-01-11 18:34:51 +00:00

change usage of tags to recipe versions in deploy.go

continuous-integration/drone/push Build is failing

Details

a51a3b57f6

Apfelwurm marked the pull request as ready for review 2026-01-11 18:38:54 +00:00

ammaratef45 commented

2026-01-11 18:41:37 +00:00

read the code and it lgtm
git fetch && git checkout fixBrokenRecipeCheckout && make build
.abra app new zammad has expected output
existing unit tests pass
change is not covered with unit tests

Amazing work @Apfelwurm, Can I nudge you to add some unit tests though?

- [x] read the code and it lgtm - [x] `git fetch && git checkout fixBrokenRecipeCheckout && make build` - [x] `.abra app new zammad` has expected output - [x] existing unit tests pass - [ ] change is not covered with unit tests Amazing work @Apfelwurm, Can I nudge you to add some unit tests though?

Apfelwurm marked the pull request as work in progress 2026-01-11 18:52:59 +00:00

Apfelwurm commented

2026-01-11 18:53:13 +00:00

gone back to WIP, to implement tests :)

decentral1se commented

2026-01-11 21:34:06 +00:00

Great stuff @Apfelwurm @ammaratef45 🔥

Some times it's a pain to unit test stuff that are in these huge main blocks. In that case, I would just make a simple integration test which does abra app new zammad and checks for the warning output.

I would generally hope that running all app_deploy*.bats integration tests (see here) would help make you feel better about whether this works or not 🙃

It might seem like a lot but you can run the entire suite locally thanks to the work of @p4u1. We did our best with the docs: https://docs.coopcloud.tech/abra/hack/#running-them-locally more eyeballs and people diving in is extremely welcome!

Great stuff @Apfelwurm @ammaratef45 🔥 Some times it's a pain to unit test stuff that are in these huge main blocks. In that case, I would just make a simple integration test which does `abra app new zammad` and checks for the warning output. I would generally hope that running all `app_deploy*.bats` integration tests (see [here](https://git.coopcloud.tech/toolshed/abra/src/branch/main/tests/integration)) would help make you feel better about whether this works or not 🙃 It might seem like a lot but you can run the entire suite locally thanks to the work of @p4u1. We did our best with the docs: https://docs.coopcloud.tech/abra/hack/#running-them-locally more eyeballs and people diving in is extremely welcome!

❤️ 2

decentral1se removed the invalid label 2026-01-11 21:36:05 +00:00

decentral1se added this to the Abra v0.13 project 2026-01-11 21:36:11 +00:00

ammaratef45 commented

2026-01-11 21:57:30 +00:00

I find it helpful to extract some of the code from the huge main block into a function to unit test, I don't know what is the norm for go developers though (this is the only project written in Go that I ever worked on lol)

decentral1se commented

2026-01-12 13:18:52 +00:00

This is basically also my first go project 😆 dog bless us all

😆 2

decentral1se referenced this pull request

2026-01-15 09:04:43 +00:00

ensure repo is up to date before printing status #753

decentral1se moved this to In Progress in Abra v0.13 on 2026-01-16 23:06:22 +00:00

ammaratef45 commented

2026-01-22 01:18:27 +00:00

@Apfelwurm any updates on this?

Apfelwurm commented

2026-01-22 01:34:50 +00:00

@Apfelwurm any updates on this?

tried it this evening, but had problems with even running both the unit as well as the integration tests in a consistent manner locally, so i gave up for the day after like 1,5h :D
I think with another try and maybe some help (@p4u1 ? <3 :D ) i will get through it. Gonna need some time though, cause currently a lot is going on :)

> @Apfelwurm any updates on this? tried it this evening, but had problems with even running both the unit as well as the integration tests in a consistent manner locally, so i gave up for the day after like 1,5h :D I think with another try and maybe some help (@p4u1 ? <3 :D ) i will get through it. Gonna need some time though, cause currently a lot is going on :)

ammaratef45 commented

2026-01-22 02:27:07 +00:00

tried it this evening, but had problems with even running both the unit as well as the integration tests in a consistent manner locally, so i gave up for the day after like 1,5h :D
I think with another try and maybe some help (@p4u1 ? <3 :D ) i will get through it. Gonna need some time though, cause currently a lot is going on :)

Cool! no rush, let me know if I can help too

> tried it this evening, but had problems with even running both the unit as well as the integration tests in a consistent manner locally, so i gave up for the day after like 1,5h :D > I think with another try and maybe some help (@p4u1 ? <3 :D ) i will get through it. Gonna need some time though, cause currently a lot is going on :) Cool! no rush, let me know if I can help too

Apfelwurm force-pushed fixBrokenRecipeCheckout from a51a3b57f6 to 58b7891219

2026-02-06 11:04:41 +00:00

Compare

Apfelwurm force-pushed fixBrokenRecipeCheckout from 58b7891219 to c08314e1cc

2026-02-06 11:06:08 +00:00

Compare

decentral1se commented

2026-02-07 18:25:35 +00:00

but had problems with even running both the unit as well as the integration tests in a consistent manner locally, so i gave up for the day after like 1,5h :D

Oh dear, let us know how we can help!

Everything seems to be passing atm... https://build.coopcloud.tech/toolshed/abra/3538

> but had problems with even running both the unit as well as the integration tests in a consistent manner locally, so i gave up for the day after like 1,5h :D Oh dear, let us know how we can help! Everything seems to be passing atm... https://build.coopcloud.tech/toolshed/abra/3538

javielico commented

2026-02-08 17:53:15 +00:00

First-time contributor

thank you @Apfelwurm for pointing me to this issue, I will keep an eye on it and once it's resolved and I will try and push the updates for zammad on abra.

👍 1

Apfelwurm commented

2026-02-11 02:37:23 +00:00

thank you @Apfelwurm for pointing me to this issue, I will keep an eye on it and once it's resolved and I will try and push the updates for zammad on abra.

ah, just seeing this message here :D
you can either do that, or just quickly clone the abra repo, check out the fixBrokenRecipeCheckout branch and run make build && make install to use it. The changes are working and should not break anything else, the only thing missing are the tests as of right now :)

> thank you @Apfelwurm for pointing me to this issue, I will keep an eye on it and once it's resolved and I will try and push the updates for zammad on abra. ah, just seeing this message here :D you can either do that, or just quickly clone the abra repo, check out the fixBrokenRecipeCheckout branch and run `make build && make install` to use it. The changes are working and should not break anything else, the only thing missing are the tests as of right now :)

Apfelwurm force-pushed fixBrokenRecipeCheckout from c08314e1cc to d155333e20

2026-02-13 14:32:56 +00:00

Compare

Apfelwurm added 1 commit 2026-02-13 16:32:42 +00:00

add integration test that does not use old recipe version when recipe is broken

continuous-integration/drone/push Build is failing

Details

d07b775581

Apfelwurm marked the pull request as ready for review 2026-02-13 16:33:23 +00:00

Apfelwurm commented

2026-02-13 16:36:45 +00:00

finnaly looked into the integration tests together with @p4u1 and we wrote this simple one based on the zammad recipe.
We tried to custom build a broken test recipe for the case, but somehow we could not reproduce the initial problemetic behaviour there, which is quiet strange, but i think this is a result of some cascading bugs which i don't understand yet :'D

Non the less, this PR does what its supposed to and fixes zammad deployment :)

PS: For unit tests the time is currently sadly to sparse

finnaly looked into the integration tests together with @p4u1 and we wrote this simple one based on the zammad recipe. We tried to custom build a broken test recipe for the case, but somehow we could not reproduce the initial problemetic behaviour there, which is quiet strange, but i think this is a result of some cascading bugs which i don't understand yet :'D Non the less, this PR does what its supposed to and fixes zammad deployment :) PS: For unit tests the time is currently sadly to sparse

🎉 1

decentral1se approved these changes 2026-02-13 21:51:43 +00:00

decentral1se left a comment

🥇

decentral1se merged commit 8f7dbfedbc into main

2026-02-13 21:51:48 +00:00

decentral1se deleted branch fixBrokenRecipeCheckout

2026-02-13 21:51:48 +00:00

decentral1se commented

2026-02-14 17:34:45 +00:00

@Apfelwurm the test seems to be failing on the nightly CI

https://build.coopcloud.tech/toolshed/abra/3560/1/7

@Apfelwurm the test seems to be failing on the nightly CI > https://build.coopcloud.tech/toolshed/abra/3560/1/7

😕 1

Apfelwurm commented

2026-02-14 22:45:16 +00:00

@Apfelwurm the test seems to be failing on the nightly CI

https://build.coopcloud.tech/toolshed/abra/3560/1/7

Oh no :/ cant reproduce this locally, is this maybe a timeout issue in the recipe? we could theoretically remove the assert_success, since it is not 100% required to make sure that the fix does its job, since the used version is what we care about, but that feels kind of ugly :D Is there any easy way to get the logs? maybe we should also consider to implement some mechanism, that automatically prints the abra logs contents when executed in the ci?

> @Apfelwurm the test seems to be failing on the nightly CI > > > https://build.coopcloud.tech/toolshed/abra/3560/1/7 Oh no :/ cant reproduce this locally, is this maybe a timeout issue in the recipe? we could theoretically remove the assert_success, since it is not 100% required to make sure that the fix does its job, since the used version is what we care about, but that feels kind of ugly :D Is there any easy way to get the logs? maybe we should also consider to implement some mechanism, that automatically prints the abra logs contents when executed in the ci?

👀 1

decentral1se commented

2026-02-15 13:39:23 +00:00

Ah fuck it, let's just do 7c3364f87a 🤸‍♀️

Ah fuck it, let's just do 7c3364f87ab542b43ec145aba43659c23a8c1d36 🤸‍♀️

😆 2

decentral1se moved this to Done in Abra v0.13 on 2026-02-15 13:47:42 +00:00

ammaratef45 commented

2026-02-15 18:10:31 +00:00

huh, what does --no-converge-checks do?

huh, what does `--no-converge-checks` do?

Sign in to join this conversation.