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

internal/dag,envoy: use constant for ca.crt #2327

Merged
merged 1 commit into from
Mar 10, 2020

Conversation

tsaarni
Copy link
Member

@tsaarni tsaarni commented Mar 9, 2020

Cleaning up locally defined strings when referencing to secrets with
CA certificates and replaced them with a single constant.

Signed-off-by: Tero Saarni tero.saarni@est.tech

@tsaarni
Copy link
Member Author

tsaarni commented Mar 9, 2020

This clean up was proposed by @jpeach in review comment #2250 (comment)

@codecov
Copy link

codecov bot commented Mar 9, 2020

Codecov Report

Merging #2327 into master will not change coverage.
The diff coverage is 80%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2327   +/-   ##
======================================
  Coverage    77.9%   77.9%           
======================================
  Files          59      59           
  Lines        5205    5205           
======================================
  Hits         4055    4055           
  Misses       1062    1062           
  Partials       88      88
Impacted Files Coverage Δ
internal/envoy/cluster.go 100% <100%> (ø) ⬆️
internal/dag/cache.go 95.43% <100%> (ø) ⬆️
internal/dag/builder.go 92.67% <100%> (ø) ⬆️
internal/dag/secret.go 60.22% <50%> (ø) ⬆️

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 f056a4b...a75b424. Read the comment docs.

@jpeach jpeach self-requested a review March 9, 2020 20:23
@jpeach jpeach self-assigned this Mar 9, 2020
Copy link
Contributor

@jpeach jpeach left a comment

Choose a reason for hiding this comment

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

LGTM, just a nit to improve the comment.

internal/dag/dag.go Outdated Show resolved Hide resolved
internal/dag/dag.go Outdated Show resolved Hide resolved
Cleaning up locally defined strings when referencing to secrets with
CA certificate bundles and replaced them with a single constant.

Signed-off-by: Tero Saarni <tero.saarni@est.tech>
@tsaarni tsaarni force-pushed the refactor-cacrt-constant branch from 0c91dd5 to a75b424 Compare March 10, 2020 05:25
Copy link
Contributor

@jpeach jpeach left a comment

Choose a reason for hiding this comment

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

Nice work, thanks!

@jpeach jpeach merged commit 20fe2c8 into projectcontour:master Mar 10, 2020
@tsaarni tsaarni deleted the refactor-cacrt-constant branch March 10, 2020 06:30
@jpeach jpeach added this to the 1.3.0 milestone Mar 20, 2020
@tsaarni tsaarni restored the refactor-cacrt-constant branch March 26, 2020 12:23
@tsaarni tsaarni deleted the refactor-cacrt-constant branch March 26, 2020 12:26
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.

2 participants