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.

Sign up for GitHub

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

Jump to bottom

fix external redis existing secret url render #1614

Merged
merged 1 commit into from
Nov 2, 2023
Merged

fix external redis existing secret url render #1614

merged 1 commit into from
Nov 2, 2023

Conversation

MinerYang
Copy link
Collaborator

cherry-pick of : #1607
fix: goharbor/harbor#19387

@Kajot-dev
Copy link
Contributor

@MinerYang This PR contains a bug, it will not use redis existingSecret for registry component (registry component uses "harbor.redis.password" which still does not use existingSecret

@MinerYang
Copy link
Collaborator Author

MinerYang commented Oct 18, 2023 •
edited
Loading

@MinerYang This PR contains a bug, it will not use redis existingSecret for registry component (registry component uses "harbor.redis.password" which still does not use existingSecret

registry will use password from registry env

harbor-helm/templates/registry/registry-dpl.yaml

Line 224 in 39befea

{{- if .Values.redis.external.existingSecret }}

@Kajot-dev
Copy link
Contributor

That's registryctl container, but not the registry container. I deployed version of the chart with this PR and caching in redis enabled and registry container was getting NOAUTH from redis, while registryctl was not

@MinerYang
fix external redis existing secret url render
7b6501a 
Signed-off-by: yminer <[email protected]>

update redis password for registry conatiner

Signed-off-by: yminer <[email protected]>
@MinerYang MinerYang force-pushed the cp-fix-external-redis-secret-render branch from 291a73d to 7b6501a Compare October 24, 2023 15:03
@MinerYang
Copy link
Collaborator Author

MinerYang commented Oct 25, 2023 •
edited
Loading

That's registryctl container, but not the registry container. I deployed version of the chart with this PR and caching in redis enabled and registry container was getting NOAUTH from redis, while registryctl was not

Thanks @Kajot-dev! Sorry for the late response and it indeed another issue introduced by #1179. Updated and Have it fixed.

@MinerYang MinerYang merged commit aa2569d into goharbor:1.13.0 Nov 2, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zyyw zyyw zyyw approved these changes

@ywk253100 ywk253100 ywk253100 approved these changes

Assignees
No one assigned
Labels
target/1.13.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MinerYang @Kajot-dev @ywk253100 @zyyw