Detect major dependency version bumps #571

2023-11-18T13:27:41Z

justusbunsi commented

2023-11-18 13:27:41 +00:00

(Migrated from gitea.com)

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 (Migrated from gitea.com) reviewed 2023-11-18 13:31:04 +00:00

Makefile

						
				@ -10,3 +10,3 @@

				.PHONY: unittests

				unittests:

					helm unittest --strict -f 'unittests/**/*.yaml' -f 'unittests/dependency-major-image-check.yaml' -f 'unittests/values-conflicting-checks.yaml' ./

					helm unittest --strict -f 'unittests/**/*.yaml' -f 'unittests/dependency-major-image-check.yaml' ./

justusbunsi (Migrated from gitea.com) commented

2023-11-18 13:31:04 +00:00

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¹ which obscures the test results and failures in our builds.

https://github.com/helm-unittest/helm-unittest/issues/237 ↩︎

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 commented

2023-11-18 13:35:45 +00:00

(Migrated from gitea.com)

Major-change failures look like this:

Major-change failures look like this: ![image](/attachments/cb122f51-aa55-41b6-a2c8-5c5ee4494853)

pat-s (Migrated from gitea.com) reviewed 2023-11-18 15:20:13 +00:00

pat-s (Migrated from gitea.com) approved these changes 2023-11-18 17:05:09 +00:00

pat-s (Migrated from gitea.com) left a comment

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

(Migrated from gitea.com)

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

(Migrated from gitea.com)

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.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lunny/helm-chart#571