gitea/helm-chart
45
54
Fork 126
Code Issues 32 Pull Requests 9 Actions Releases 87 Activity

Detect major dependency version bumps #571

Merged
justusbunsi merged 6 commits from justusbunsi/helm-chart:409-detect-major-version-bumps into main 2023-11-27 18:36:47 +00:00
Conversation 3 Commits 6 Files Changed 2 +43 -1
justusbunsi commented 2023-11-18 13:27:41 +00:00
Member
Copy Link

As seen in #507 and #569, there is no guarantee for us that minor
dependency updates are actually minor updates for the dependent
application itself. The Chart version might be minor - and therefore
automatically merged when build is green - but the used Docker image
inside the Chart could still be a major version change.

To effectively prevent such automerge when the application major version
changes, there is now a test file that has the currently used major
versions hard-coded. In case of an actual major bump, this file has to
be adjusted.

Looking at redis-cluster, there might be several major Chart versions
with the same major application version.

This PR is related to #409 but does not fully resolve it.

As seen in #507 and #569, there is no guarantee for us that minor dependency updates are actually minor updates for the dependent application itself. The Chart version might be minor - and therefore automatically merged when build is green - but the used Docker image inside the Chart could still be a major version change. To effectively prevent such automerge when the application major version changes, there is now a test file that has the currently used major versions hard-coded. In case of an actual major bump, this file has to be adjusted. Looking at `redis-cluster`, there might be several major Chart versions with the same major application version. This PR is related to #409 but does not fully resolve it.
justusbunsi added 1 commit 2023-11-18 13:27:41 +00:00
Detect major dependency version bumps 
As seen in #507 and #569, there is no guarantee for us that minor
dependency updates are actually minor updates for the dependent
application itself. The Chart version might be minor - and therefore
automatically merged when build is green - but the used Docker image
inside the Chart could still be a major version change.

To effectively prevent such automerge when the application major version
changes, there is now a test file that has the currently used major
versions hard-coded. In case of an actual major bump, this file has to
be adjusted.

Looking at `redis-cluster`, there might be several major Chart versions
with the same major application version.

This PR is related to #409 but does not fully resolve it.

Signed-off-by: justusbunsi <sk.bunsenbrenner@gmail.com>
All checks were successful
check-and-test / check-and-test (pull_request) Successful in 30s
Details
e4da2e1b6d
justusbunsi reviewed 2023-11-18 13:31:04 +00:00
Makefile
@ -10,3 +10,3 @@
.PHONY: unittests
unittests:
helm unittest --strict -f 'unittests/**/*.yaml' ./
helm unittest --strict -f 'unittests/**/*.yaml' -f 'unittests/dependency-major-image-check.yaml' ./
justusbunsi commented 2023-11-18 13:31:04 +00:00
Author
Member
Copy Link

I added the major image check at the end of the suite so that it would be the last test file to run. The current v3.0.6 of helm-unittest suffers from an excessive log output1 which obscures the test results and failures in our builds.

I added the major image check at the end of the suite so that it would be the last test file to run. The current `v3.0.6` of helm-unittest suffers from an excessive log output[^1] which obscures the test results and failures in our builds. [^1]: https://github.com/helm-unittest/helm-unittest/issues/237
justusbunsi added 1 commit 2023-11-18 13:32:06 +00:00
Intentionally break the build to show how it looks 
Signed-off-by: justusbunsi <sk.bunsenbrenner@gmail.com>
Some checks failed
check-and-test / check-and-test (pull_request) Failing after 7s
Details
bb9cb26686
justusbunsi added 1 commit 2023-11-18 13:33:57 +00:00
Intentionally break the build - part 2 
Signed-off-by: justusbunsi <sk.bunsenbrenner@gmail.com>
Some checks failed
check-and-test / check-and-test (pull_request) Failing after 29s
Details
7f57151c54
justusbunsi commented 2023-11-18 13:35:45 +00:00
Author
Member
Copy Link

Major-change failures look like this:

image

Major-change failures look like this: ![image](/attachments/cb122f51-aa55-41b6-a2c8-5c5ee4494853)
image.png
62 KiB
justusbunsi added 2 commits 2023-11-18 13:36:31 +00:00
Revert "Intentionally break the build to show how it looks" 
This reverts commit bb9cb26686.
All checks were successful
check-and-test / check-and-test (pull_request) Successful in 31s
Details
a1fe73bbb7
justusbunsi added the
skip-changelog
 label 2023-11-18 13:47:26 +00:00
justusbunsi changed title from WIP: Detect major dependency version bumps to Detect major dependency version bumps 2023-11-18 15:20:06 +00:00
justusbunsi requested review from pat-s 2023-11-18 15:20:13 +00:00
pat-s approved these changes 2023-11-18 17:05:09 +00:00
pat-s left a comment
Member
Copy Link

Awesome, thanks!

Looking at redis-cluster, there might be several major Chart versions
with the same major application version.

Yes, if there are major changes in the chart, there will be a major version bump as well. So it could be for both reasons, major application version update or chart version.

Good to have the security now to prevent issues like #507!

I also thought in more detail how we could go with the bitnami chart updates, i.e. we could ensure staying on the latest chart version but encouraging users to fix their image tag to a version compatible with their actual PG version.

This has two benefits:

  • new users always start with the latest major application version
  • Users can decide on their own when they do the upgrade
  • We don't fall behind chart changes

But more on this soon...

Awesome, thanks! > Looking at redis-cluster, there might be several major Chart versions with the same major application version. Yes, if there are major changes in the chart, there will be a major version bump as well. So it could be for both reasons, major application version update or chart version. Good to have the security now to prevent issues like #507! I also thought in more detail how we could go with the bitnami chart updates, i.e. we could ensure staying on the latest chart version but encouraging users to fix their image tag to a version compatible with their actual PG version. This has two benefits: - new users always start with the latest major application version - Users can decide on their own when they do the upgrade - We don't fall behind chart changes But more on this soon...
pat-s commented 2023-11-18 17:06:12 +00:00
Member
Copy Link

With this PR merged, are we ready to enable branch automerging for minor & friends?

And do we really want to "skip changelog" for such features? Sure they are chart internal but I stil think they are worth being mentioned in a changelog.

With this PR merged, are we ready to enable branch automerging for minor & friends? And do we really want to "skip changelog" for such features? Sure they are chart internal but I stil think they are worth being mentioned in a changelog.
justusbunsi commented 2023-11-18 17:18:22 +00:00
Author
Member
Copy Link

Good point on the labels. Let's remove them on both my PRs from today.

Regarding automerge: From what I see there are still some things to ensure for #409 really be resolved. E.g that our mapping for ports has an effect. In general, all subcharts settings we have in our values should be verified being templated correctly. We had regressions in the past I would like to prevent that for the future. Otherwise we always go through the subchart changes ourself to see what changed.
If users add their own subchart values, that's out of our scope. Those we set are our responsibility to check.

Good point on the labels. Let's remove them on both my PRs from today. Regarding automerge: From what I see there are still some things to ensure for #409 really be resolved. E.g that our mapping for ports has an effect. In general, all subcharts settings _we_ have in our values should be verified being templated correctly. We had regressions in the past I would like to prevent that for the future. Otherwise we always go through the subchart changes ourself to see what changed. If users add their own subchart values, that's out of our scope. Those _we_ set are our responsibility to check.
justusbunsi removed the
skip-changelog
 label 2023-11-18 17:18:34 +00:00
justusbunsi added 1 commit 2023-11-27 18:35:57 +00:00
Merge branch 'main' into 409-detect-major-version-bumps
All checks were successful
check-and-test / check-and-test (pull_request) Successful in 29s
Details
42ea751c46
justusbunsi merged commit 8bcd2dc63b into main 2023-11-27 18:36:47 +00:00
justusbunsi deleted branch 409-detect-major-version-bumps 2023-11-27 18:36:48 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
pat-s
Labels
Clear labels   
has
backport

  
in progress

   
invalid
  
kind
breaking

  
kind
bug

  
kind
build

  
kind
dependency

  
kind
deployment

  
kind
docs

  
kind
enhancement

  
kind
feature

  
kind
lint

  
kind
proposal

  
kind
question

  
kind
refactor

  
kind
security

  
kind
testing

  
kind
translation

  
kind
ui

  
need
backport

  
priority
critical

  
priority
low

  
priority
maybe

  
priority
medium

  
reviewed
duplicate

  
reviewed
invalid

  
reviewed
wontfix

  
skip-changelog
  
status
blocked

  
status
needs-feedback

  
status
needs-reviews

  
status
wip

  
upstream
gitea

  
upstream
other

No Label
has
backport
in progress
invalid
kind
breaking
kind
bug
kind
build
kind
dependency
kind
deployment
kind
docs
kind
enhancement
kind
feature
kind
lint
kind
proposal
kind
question
kind
refactor
kind
security
kind
testing
kind
translation
kind
ui
need
backport
priority
critical
priority
low
priority
maybe
priority
medium
reviewed
duplicate
reviewed
invalid
reviewed
wontfix
skip-changelog
status
blocked
status
needs-feedback
status
needs-reviews
status
wip
upstream
gitea
upstream
other
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: gitea/helm-chart#571
No description provided.
Delete Branch "justusbunsi/helm-chart:409-detect-major-version-bumps"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?