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

chore(security-apps): Fail deprecated gangway installations #1366

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

hairmare
Copy link
Contributor

@hairmare hairmare commented Jan 16, 2025

Description

Gangway has been marked as deprecated for more than 6 months and now is the time to make it fail.

Issues

Checklist

  • This PR contains a description of the changes I'm making
  • I updated the version in Chart.yaml
  • I updated the changelog with an artifacthub.io/changes annotation in Chart.yaml, check the example in the documentation.
  • I updated applicable README.md files using pre-commit run
  • I documented any high-level concepts I'm introducing in docs/
  • CI is currently green and this is ready for review
  • I am ready to test changes after they are applied and released

@github-actions github-actions bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 16, 2025
@hairmare hairmare marked this pull request as ready for review January 16, 2025 16:14
@hairmare hairmare requested a review from a team as a code owner January 16, 2025 16:14
@hairmare hairmare requested review from cfi2017 and vmaillot January 16, 2025 16:14
@hairmare hairmare force-pushed the chore/security-apps/fail-gangway-as-deprecated branch from da243a5 to 8236801 Compare January 16, 2025 17:27
@@ -2,7 +2,7 @@ apiVersion: v2
name: security-apps
description: Argo CD app-of-apps config for security applications
type: application
version: 0.89.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo this should be a minor release, not a patch release

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Else if someone is using 0.89.* their deployment may fail without user interaction

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there still folks that do have x.z.*, i thought we deprecated that ages ago

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a public chart, so we may not be the only consumers. It's also a breaking change in a patch version. A minor version is more of a signal that "hey, we may need to change something" for consumers so I believe it's a better fit here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review, i finally got to updating the PR to bump the minor and not the patch version.

I totally agree with you in principal, the main reason i asked was because we used to employ the x.y.* all over the place and we started retiring it's use a few years back.

@vmaillot
Copy link
Contributor

@hairmare hairmare force-pushed the chore/security-apps/fail-gangway-as-deprecated branch from 8236801 to 9861a79 Compare January 28, 2025 18:51
@github-actions github-actions bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 28, 2025
@hairmare hairmare force-pushed the chore/security-apps/fail-gangway-as-deprecated branch 2 times, most recently from 9296af4 to 0b6ebf0 Compare January 28, 2025 18:55
@github-actions github-actions bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 28, 2025
@hairmare hairmare force-pushed the chore/security-apps/fail-gangway-as-deprecated branch from 0b6ebf0 to 16d7ca1 Compare January 28, 2025 18:56
@hairmare hairmare requested a review from cfi2017 January 28, 2025 19:12
Copy link
Contributor

@cfi2017 cfi2017 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@hairmare hairmare enabled auto-merge (squash) January 28, 2025 19:31
@hairmare hairmare force-pushed the chore/security-apps/fail-gangway-as-deprecated branch from 16d7ca1 to a6aa949 Compare January 28, 2025 19:31
@hairmare hairmare merged commit b92c6bb into main Jan 28, 2025
3 checks passed
@hairmare hairmare deleted the chore/security-apps/fail-gangway-as-deprecated branch January 28, 2025 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants