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

MySQL Scaler: don't expose connection string in metricName #2171

Merged
merged 7 commits into from
Oct 13, 2021
Merged

MySQL Scaler: don't expose connection string in metricName #2171

merged 7 commits into from
Oct 13, 2021

Conversation

zroubalik
Copy link
Member

@zroubalik zroubalik commented Oct 7, 2021 •
edited
Loading

Signed-off-by: Zbynek Roubalik zroubali@redhat.com

Checklist

  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • Changelog has been updated

Fixes #2165

@zroubalik zroubalik requested a review from ahmelsayed as a code owner October 7, 2021 16:19
@zroubalik zroubalik added this to the v2.5.0 milestone Oct 12, 2021
arschles
arschles reviewed Oct 12, 2021
View reviewed changes
Copy link
Contributor

@arschles arschles left a comment

Choose a reason for hiding this comment

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

Looks good - my only thought would be to add a test, if possible

@zroubalik zroubalik changed the title MySQL Scaler: masking connection string in metricName MySQL Scaler: don't expose connection string in metricName Oct 13, 2021
@zroubalik
Copy link
Member Author

zroubalik commented Oct 13, 2021 •
edited
Loading

I've changed the approach a little bit and omitted connection string from the metric name, because we have landed #2161 and it should be enough to ensure uniqueness.

Zbynek Roubalik added 5 commits October 13, 2021 12:05
MySQL Scaler: masking connection string in metricName
4b4092d 
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
update changelog
91579d4 
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
don't use connection string in metricName
1ac9d83 
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
update changelog
5316b74 
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
incorporate recent changes
65248cf 
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
@zroubalik
Copy link
Member Author

@JorTurFer PTAL

JorTurFer
JorTurFer approved these changes Oct 13, 2021
View reviewed changes
pkg/scalers/mysql_scaler.go Outdated Show resolved Hide resolved
remove meta.scalerIndex
bfb087d 
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
@zroubalik zroubalik merged commit 303d2bc into kedacore:main Oct 13, 2021
nilayasiktoprak pushed a commit to nilayasiktoprak/keda that referenced this pull request Oct 23, 2021
@zroubalik @nilayasiktoprak
MySQL Scaler: don't expose connection string in metricName (kedacor…
c5021f9 
…e#2171)

Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
Signed-off-by: nilayasiktoprak <nilayasiktoprak@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arschles arschles arschles left review comments

@JorTurFer JorTurFer JorTurFer approved these changes

@tomkerkhove tomkerkhove tomkerkhove approved these changes

@ahmelsayed ahmelsayed Awaiting requested review from ahmelsayed

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.5.0
Development

Successfully merging this pull request may close these issues.

MySQL Scaler: Secrets openly shown in metric name
4 participants
@zroubalik @arschles @tomkerkhove @JorTurFer