fix: keep the release a minor one - #44
Merged
Merged
Conversation
Skipping pre-release tags was not asked for by the audit and stops a repository that releases through -rc tags from deploying at all. Choosing what to tag is the operator's decision, and a pre-release still has to pass quorum verification like any other tag, so trx no longer second-guesses it. Reverts the behaviour to the 1.0.0 one and removes the allowPrerelease option introduced with it. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
Recording the failed tag stopped the natural way out of a failure whose cause is outside the repository: a deployment that failed because the cluster was unreachable no longer recovered by itself once the cluster came back, it needed a new tag or a manual --force. Reverts the skip to the 1.0.0 behaviour. The prune added with it stays: a tag deleted or moved upstream still stops winning tag selection. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
The key was read into a field nothing used, and removing it as dead code made every config that still carries it fail to load, because decoding rejects unknown keys. It is accepted and ignored again, and marked deprecated in favour of repo.initialLastProcessedTag. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
A quorum listing the same key twice used to be a config error, so a deployment that had been running with such a config stopped starting at all after the fix. The duplicate is now dropped from the trusted keys of the quorum and logged: verification needs as many distinct key holders as before, and a quorum that can no longer be satisfied fails through onQuorumFailure, where the operator is already listening. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
The 2.0.0 the release PR proposes comes from a BREAKING CHANGE trailer in ad9f231, which claims that verification stops accepting DSA and ElGamal keys and SHA-1 signatures after the move to ProtonMail/go-crypto. That policy lives in the openpgp/v2 API; trdl, and therefore trx, uses v1, where it is never applied. A DSA-signed tag and a SHA-1 signed tag both still pass CheckQuorums on this branch. The changes that really were breaking are reverted in this branch, so the range from v1.0.0 is a minor release. Release-As: 1.1.0 Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes the range from
v1.0.0a minor release by reverting the changes that really were breaking, and retargets the pending release PR withRelease-As: 1.1.0.The breaking change the release PR is based on does not exist
ad9f231carriesBREAKING CHANGE: signature verification now rejects DSA and ElGamal keys and signatures hashed with SHA-1 or MD5, which is what makes release-please propose 2.0.0.The verification path is
quorum.CheckQuorums→trdl/server/pkg/git.VerifyTagSignatures→trdl/server/pkg/pgp/util.go→openpgp.CheckArmoredDetachedSignature(keyring, …, nil). The library did move fromgolang.org/x/crypto/openpgptoProtonMail/go-crypto, butdefaultRejectPublicKeyAlgorithms(ElGamal, DSA) anddefaultRejectMessageHashAlgorithms(SHA-1, MD5, RIPEMD160) are only consulted from theopenpgp/v2API —v2/keys.go:793,v2/read.go:739,v2/read.go:762. trdl uses the v1 API, where nothing reads them.Checked against this branch with real keys and real signed tags, through
CheckQuorums:Reverted
repo.allowPrereleaseis gone with the skip. Picking what to tag is the operator's decision, and a pre-release still has to pass quorum verification. It was never an audit finding.--forceafterwards. The fetch prune added alongside it stays.Kept, but no longer breaking
initial_last_published_git_commitis accepted and ignored again. Nothing ever read it, but decoding rejects unknown keys, so removing the field stopped every config carrying it from loading.onQuorumFailure, where the operator is already listening, instead of trx refusing to start.Still changed since v1.0.0, deliberately
onQuorumFailureandonCommandSkippedrun in trx's own working directory rather than in the checkout (the #16 fix — a relative./notify.shstops resolving); the operatorenvoverrides the repository one, as the README always documented; env names from a repositorytrx.yamlare upper-cased; an unknown template variable fails the run instead of rendering<no value>; the checkout removes untracked files.go vet,go test -race ./...,GOOS=windows go build ./...,gofumptclean.