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

feat: Allow AnalysisTemplates to reference secrets #420

Merged
merged 18 commits into from
Mar 4, 2020

Conversation

khhirani
Copy link
Contributor

@khhirani khhirani commented Feb 28, 2020

Modified AnalysisTemplates to allow Metric Providers to reference secrets (Resolves #372)

Created a RedactorFormatter to redact secrets from log messages

@codecov
Copy link

codecov bot commented Feb 28, 2020

Codecov Report

Merging #420 into master will increase coverage by 0.09%.
The diff coverage is 94.28%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #420      +/-   ##
=========================================
+ Coverage    84.5%   84.6%   +0.09%     
=========================================
  Files          70      71       +1     
  Lines        6667    6721      +54     
=========================================
+ Hits         5634    5686      +52     
- Misses        746     747       +1     
- Partials      287     288       +1
Impacted Files Coverage Δ
analysis/controller.go 51.28% <100%> (+0.63%) ⬆️
utils/log/log.go 100% <100%> (ø) ⬆️
utils/log/redactor_formatter.go 75% <75%> (ø)
analysis/analysis.go 85.71% <96.36%> (+1.75%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57134f7...946d247. Read the comment docs.

Copy link
Member

@dthomson25 dthomson25 left a comment

Choose a reason for hiding this comment

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

Hi @khhirani, I have some questions and minor style comments, but overall I think this PR is a really good start!

A couple things I noticed at a high level:

pkg/apis/rollouts/v1alpha1/analysis_types.go Outdated Show resolved Hide resolved
pkg/apis/rollouts/v1alpha1/analysis_types.go Show resolved Hide resolved
controller/controller.go Show resolved Hide resolved
analysis/analysis.go Outdated Show resolved Hide resolved
analysis/analysis.go Outdated Show resolved Hide resolved
analysis/analysis_test.go Outdated Show resolved Hide resolved
analysis/analysis_test.go Outdated Show resolved Hide resolved
analysis/analysis_test.go Outdated Show resolved Hide resolved
analysis/controller_test.go Outdated Show resolved Hide resolved
utils/log/log.go Outdated Show resolved Hide resolved
@khhirani khhirani changed the title feat: Allow feat: Allow AnalysisTemplates to reference secrets Mar 2, 2020
Copy link
Member

@dthomson25 dthomson25 left a comment

Choose a reason for hiding this comment

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

One minor comment, but otherwise good to go!

analysis/analysis.go Outdated Show resolved Hide resolved
Copy link
Member

@dthomson25 dthomson25 left a comment

Choose a reason for hiding this comment

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

LGTM! Great job!

@khhirani khhirani merged commit 93312ee into argoproj:master Mar 4, 2020
@khhirani khhirani deleted the webmetric-ref-secrets branch March 4, 2020 23:23
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.

Ability for web metric to reference secret content
3 participants