Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Fix YTT processing for passwords with yaml special chars #4548

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tenczar
Copy link
Contributor

@tenczar tenczar commented Apr 4, 2023

Fixes our yaml processor so that passwords are passed to ytt as string values. This fixes a bug that would occur when a special characters in password would act like yaml structures and cause parsing errors. Examples: any password starting with an ! would be empty in ytt as yaml treats a string starting with the bang as a tag, Any password ending with a colon (:) would be treated as an empty map and not a string causing parsing errors.

What this PR does / why we need it

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

Release note


Additional information

Special notes for your reviewer

@tenczar tenczar requested review from a team as code owners April 4, 2023 22:26
@github-advanced-security
Copy link

You have successfully added a new Trivy configuration .github/workflows/cve_scan.yaml:trivy_scan. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

@github-actions
Copy link

github-actions bot commented Apr 4, 2023

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4548/20230404223414/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@tenczar tenczar force-pushed the topic/ntenczar/password_bug branch from 5090ab7 to f48083e Compare April 6, 2023 17:59
@github-actions
Copy link

github-actions bot commented Apr 6, 2023

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4548/20230406180652/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Fixes our ytt processor so that password values are not passed to the yaml
parser as yaml. This prevents the password from being interpreted as
yaml, which could lead to template errors and other failures.
@tenczar tenczar force-pushed the topic/ntenczar/password_bug branch from f48083e to f32efce Compare April 6, 2023 20:18
@github-actions
Copy link

github-actions bot commented Apr 6, 2023

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4548/20230406202803/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@codecov
Copy link

codecov bot commented Apr 6, 2023

Codecov Report

Merging #4548 (f32efce) into main (7cf3455) will decrease coverage by 0.87%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #4548      +/-   ##
==========================================
- Coverage   49.78%   48.92%   -0.87%     
==========================================
  Files         453      483      +30     
  Lines       45379    47503    +2124     
==========================================
+ Hits        22594    23243     +649     
- Misses      20632    22060    +1428     
- Partials     2153     2200      +47     
Impacted Files Coverage Δ
tkg/yamlprocessor/ytt.go 73.27% <100.00%> (-0.91%) ⬇️

... and 35 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants