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

bug: DFLY_PASSWORD environment variable deprecation #2539

Merged

Conversation

mprimeaux
Copy link
Contributor

@mprimeaux mprimeaux commented Feb 4, 2024

This PR addresses #2482. If the image.tag version is equal to 1.14.0 or greater then specify the DFLY_requirepass environment variable. Otherwise use the DFLY_PASSWORD environment variable.

The primary change I made was to _pod.tpl; lines 94 through 100 here. The other files were modified as a result of executing go test -v ./... -update.

…he DFLY_requirepass environment variable. Otherwise use the DFLY_PASSWORD environment variable.

Signed-off-by: Michael Primeaux <michael.primeaux@mac.com>
@@ -123,7 +123,7 @@ spec:
- mountPath: /etc/dragonfly/tls
name: tls
env:
- name: DFLY_PASSWORD
- name: DFLY_requirepass
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add another test config (amnd golden file) for .Values.image.tag older than v1.14 so that we can verify the same?

Mostly looks good otherwise! :)

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 just pushed a commit that I believe addresses this request.

Signed-off-by: Michael Primeaux <michael.primeaux@mac.com>
@kostasrim
Copy link
Contributor

kostasrim commented Feb 5, 2024

Hi @mprimeaux we did release a new version a few hours ago so we got conflicting changes (sorry for that). Can I sincerely ask you to sync your fork and merge main. Also for convenience see the changes in: https://github.com/mprimeaux/dragonfly/commit/97982eef2cd4436bbc6fbe8e4e35327208191c39

If this is too much I am happy to take over :)

@mprimeaux
Copy link
Contributor Author

Sure. I'll sync my fork. All good.

mprimeaux and others added 2 commits February 5, 2024 14:57
Signed-off-by: Michael Primeaux <mprimeaux@users.noreply.github.com>
Signed-off-by: Michael Primeaux <michael.primeaux@mac.com>
@mprimeaux
Copy link
Contributor Author

@kostasrim Should be ready for a review.

@Pothulapati
Copy link
Contributor

Thanks @mprimeaux for fixing the issue! I pushed some small nits around naming. Will merge it once the tests pass!

@Pothulapati Pothulapati enabled auto-merge (squash) February 6, 2024 07:54
@Pothulapati Pothulapati enabled auto-merge (squash) February 6, 2024 08:11
@Pothulapati Pothulapati merged commit 951b089 into dragonflydb:main Feb 6, 2024
7 checks passed
romange pushed a commit that referenced this pull request Feb 6, 2024
* If the image.tag version is equal to 14.0.0 or greater then specify the DFLY_requirepass environment variable. Otherwise use the DFLY_PASSWORD environment variable.

Signed-off-by: Michael Primeaux <michael.primeaux@mac.com>

* Corrected version logic and added golden test

Signed-off-by: Michael Primeaux <michael.primeaux@mac.com>

* Updated golden file for TLS + image.tag

Signed-off-by: Michael Primeaux <michael.primeaux@mac.com>

* fix CI for chart linting test

* rename new test specific to password

* use v1.13.0

* update golden chart

---------

Signed-off-by: Michael Primeaux <michael.primeaux@mac.com>
Signed-off-by: Michael Primeaux <mprimeaux@users.noreply.github.com>
Co-authored-by: Tarun Pothulapati <tarun@dragonflydb.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants