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

[BUG] Version reports 1.0.21, but 1.0.18 is checked out. #55

Closed
1 task done
borntohonk opened this issue Apr 16, 2024 · 16 comments
Closed
1 task done

[BUG] Version reports 1.0.21, but 1.0.18 is checked out. #55

borntohonk opened this issue Apr 16, 2024 · 16 comments

Comments

@borntohonk
Copy link

borntohonk commented Apr 16, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

Branch: master
Commit: b8bec48d5036a4d9084d762e2d2ffca9c711f2f3
Version: 1.0.21

https://github.com/pymedusa/Medusa/releases/tag/v1.0.18 = pymedusa/Medusa@b8bec48

Expected Behavior

expected behavior is for pymedusa/Medusa@34a67cf to be checked out, not pymedusa/Medusa@b8bec48

also docker container flag seems to be unset, and there's no git within the container.

Steps To Reproduce

  1. run the container and notice version flag has been tampered with.
  2. debug the container and find no git binary.
  3. apk add git
  4. cd /app/medusa
  5. git status
  6. notice that it's not initialized, and have been tampered with, instead of being checked out and having the docker container flag which medusa supports not set.

Environment

- OS: Truenas Scale
- How docker service was installed: integrated service through kubernetes/HELM



image

CPU architecture

x86-64

Docker creation

https://github.com/truecharts/charts/tree/master/charts/stable/medusa

Container logs

the logs themselves are filled with medusa related work / downloads / processing and are worthless to this bug report.
Copy link

Thanks for opening your first issue here! Be sure to follow the relevant issue templates, or risk having this issue marked as invalid.

@Roxedus
Copy link
Member

Roxedus commented Apr 16, 2024

Where is the tamper conclusion coming from? both medusa/common.py and CHANGELOG.md claims 1.0.21.

We do indeed not use git to grab the version, and rather the source-code tarball.

@borntohonk
Copy link
Author

borntohonk commented Apr 16, 2024

It's an assumption based on incorrect commit checkout being set,
MEDUSA_COMMIT_HASH should be set to prevent this behavior.
image

ARG GIT_COMMIT
ENV MEDUSA_COMMIT_HASH $GIT_COMMIT

(the official pymedusa docker container sets this through workflows)

        build-args: |
          GIT_COMMIT=${{ github.sha }}
          GIT_BRANCH=${{ github.ref_name }}

@aptalca
Copy link
Member

aptalca commented Apr 16, 2024

We don't check out anything, we don't use git.

We just grab the github release tarball: https://github.com/linuxserver/docker-medusa/blob/master/Dockerfile#L30-L32

@Roxedus
Copy link
Member

Roxedus commented Apr 16, 2024

We do not set/use/need that environment variable at all

@borntohonk
Copy link
Author

We do not set/use/need that environment variable at all

And that leads to this behavior where the container chooses to identify as an older cached version

@aptalca
Copy link
Member

aptalca commented Apr 16, 2024

Honestly it's quite silly to rely on git commits for version checks on stable releases. It's appropriate for development releases and dev purposes but not stable releases for public consumption. I realize various FOSS projects used to do that back in the day, including for webgui based updates, but most have moved on to proper versioning for stable releases.

With that said, we don't support internal update mechanisms via the gui. We used to go out of our way to try and disable those when it used to be common. These days it's rare so we no longer bother.

In short, if you're using our docker image, don't use web gui update mechanisms. The update instructions as well as instructions for checking the app version within containers are both described in the readme:

With regards to the tampering concerns, we take those very seriously but would need a lot more info than what is currently provided to test and confirm.

@Roxedus
Copy link
Member

Roxedus commented Apr 16, 2024

That test is at best flakey.

There is no commit tied to a release on github(it can be tied to a git tag, which does have a commit associated).
The fallback is to look for a .dockerenv file, which does indeed exist in a docker environment.
image

@borntohonk
Copy link
Author

The issue here is i use persistent storage mounted (medusa configuration) and that has indexed commit derived from prior version set and cached that.

I am using the container :latest container 1:1. One needs to set MEDUSA_COMMIT_HASH otherwise the pymedusa container will insist there are X amount of commits and in need of an update.

@aptalca
Copy link
Member

aptalca commented Apr 16, 2024

Isn't the indexed commit stored in the config file? Can't you just remove it as it never should have been set in our container to begin with? It's a side effect of migrating from either a different image or bare metal to our container and should be part of the migration steps.

@borntohonk
Copy link
Author

borntohonk commented Apr 16, 2024

You're basically suggesting I go do surgery to my config file every iteration of the container that edits common.py to change the version to a higher increment, instead of just setting the enviroment variable to prevent this behavior from happening?
edit:
just a note: it's more than "just a config file"; it's a whole system of database entries, to label it as "just a config file" is underplaying what needs to be done.

@Roxedus
Copy link
Member

Roxedus commented Apr 16, 2024

We haven't exposed the commit SHA for years in Medusa. So how it even knew which to put there is a mystery.

The time for that 43rd commit in Medusa tracks with when Truecharts decided to be less shitty by rebranding containers, who knows what they did to the images, maybe they set the env once.

@borntohonk
Copy link
Author

a live test of my database is that it bricked editing a config file to set the commit hash, and reapplying backups don't work. It's a fickle webserver from a stagnant codebase. The container itself runs.

@aptalca
Copy link
Member

aptalca commented Apr 16, 2024

I'm in a meeting and I haven't used or looked into Medusa in a loong time so I'm going by what you said.

The issue here is i use persistent storage mounted (medusa configuration) and that has indexed commit derived from prior version set and cached that.

I'm assuming you're talking about a migration related issue. Because when I put up a container just now, an update check tells me I'm already on latest and it also tells me the docker message Rox posted above.

Also like Rox said, we are not the ones publishing helm charts. Who knows what they did in the past.

@borntohonk
Copy link
Author

I'm in a meeting and I haven't used or looked into Medusa in a loong time so I'm going by what you said.

The issue here is i use persistent storage mounted (medusa configuration) and that has indexed commit derived from prior version set and cached that.

I'm assuming you're talking about a migration related issue. Because when I put up a container just now, an update check tells me I'm already on latest and it also tells me the docker message Rox posted above.

Also like Rox said, we are not the ones publishing helm charts. Who knows what they did in the past.

oh well, my database is already lost so guess i have to rebuild new anyway and assume this is a former truecharts thing, and just reopen a new issue if it turns out not to be.

@LinuxServer-CI LinuxServer-CI moved this from Issues to Done in Issue & PR Tracker Apr 16, 2024
@aptalca
Copy link
Member

aptalca commented Apr 16, 2024

For future reference (while I have the container up)

Here's the help and info page that shows the commit as unknown and the version correctly as 1.0.21. Also shows Runs in Docker: Yes
Screenshot 2024-04-16 104730

Also, /config/config.ini lists:

[General]
git_token = ""
git_reset = 1
git_reset_branches = ,
branch = master
git_remote = origin
git_remote_url = https://github.com/pymedusa/Medusa
cur_commit_hash = ""
cur_commit_branch = master
config_version = 12

What I was trying to tell you earlier is to reset that option cur_commit_hash = "" in there, which should never be set in our container. And if set due to migration, would need to be reset, otherwise you'll get incorrect update info in the webgui.

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

No branches or pull requests

3 participants