search

mozilla / addons

☂ Umbrella repository for Mozilla Addons ✨
Other
125 stars 41 forks source link

[Bug]: Enforce branch validation rules on addons-server #14907

Closed KevinMind closed 1 month ago

KevinMind commented 2 months ago

What happened?

When a branch contains invalid docker version characters like / or : it breaks tagging that relies on this information.

Ex: https://app.circleci.com/pipelines/github/mozilla/addons-server/33002/workflows/ec04dd9c-a8ff-4953-b12b-5559ce671344/jobs/217751

What did you expect to happen?

Github should prevent pushing branches that are not also valid docker image versions. This way we don't have to do any kind of validation/transformation in our CI.

Is there an existing issue for this?

┆Issue is synchronized with this Jira Task

diox commented 1 month ago

We probably do want / though: it's a common way of naming branches, and it's used by dependabot among other things.

diox commented 1 month ago

We should do a find-replace to work around this, cause we definitely want the /. Also we want to avoid latest as the branch name. @KevinMind to add the documentation about valid names.

KevinMind commented 1 month ago

Found another bug investigating this. We need to modify the source of the branch name for PRs.

push: github.ref_name pull_request: github.head_ref

We should consider adding the version to the context action so we can make the version consistent.

KevinMind commented 1 month ago

Docker allows alpha numeric characters, underscores, periods and hyphens.

https://docs.docker.com/reference/cli/docker/image/tag/

KevinMind commented 1 month ago

FWIW we can configure dependabot to use hyphens instead of slashes. https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#pull-request-branch-nameseparator

IMO the simplest solution is to limit branch names to be valid tags, then there is no mapping needed.. @diox Can you give a reason why we should allow other special characters other than "we already do it"? Is there a use case that is common and necessary?

diox commented 1 month ago

I don't mind that if we have a solution for dependabot.

KevinMind commented 1 month ago

Oddly enough, seems that docker meta action is converting the slashes to hyphens.. this is odd.

https://github.com/mozilla/addons-server/actions/runs/10269727560/job/28415844005

I want to figure out why this is happening but could be the problem is already solved. Would still like to filter out some obvious ones like tags and latest but otherwise might be okay.