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

fix(upgrade-job): avoid quotes when setting yaml bools, null, int #334

Merged
merged 1 commit into from
Aug 29, 2023
Merged

fix(upgrade-job): avoid quotes when setting yaml bools, null, int #334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 12 additions & 17 deletions k8s/upgrade/src/bin/upgrade-job/helm/values.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ pub(crate) fn generate_values_yaml_file(
if from_values.io_engine_log_level().eq(log_level_to_replace)
&& to_values.io_engine_log_level().ne(log_level_to_replace)
{
yq.set_string_value(
yq.set_value(
YamlKey::try_from(".io_engine.logLevel")?,
to_values.io_engine_log_level(),
upgrade_values_file.path(),
Expand All @@ -90,22 +90,17 @@ pub(crate) fn generate_values_yaml_file(
.eventing_enabled()
.ne(&to_values.eventing_enabled())
{
let value_as_str = match to_values.eventing_enabled() {
true => "true",
false => "false",
};

yq.set_string_value(
yq.set_value(
YamlKey::try_from(".eventing.enabled")?,
value_as_str,
to_values.eventing_enabled(),
upgrade_values_file.path(),
)?;
}

// Default options.
// Image tag is set because the high_priority file is the user's source options file.
// The target's image tag needs to be set for PRODUCT upgrade.
yq.set_string_value(
yq.set_value(
YamlKey::try_from(".image.tag")?,
to_values.image_tag(),
upgrade_values_file.path(),
Expand All @@ -114,44 +109,44 @@ pub(crate) fn generate_values_yaml_file(
// RepoTags fields will be set to empty strings. This is required because we are trying
// to get to the target container images, without setting versions for repo-specific
// components.
yq.set_string_value(
yq.set_value(
YamlKey::try_from(".image.repoTags.controlPlane")?,
"",
upgrade_values_file.path(),
)?;
yq.set_string_value(
yq.set_value(
YamlKey::try_from(".image.repoTags.dataPlane")?,
"",
upgrade_values_file.path(),
)?;
yq.set_string_value(
yq.set_value(
YamlKey::try_from(".image.repoTags.extensions")?,
"",
upgrade_values_file.path(),
)?;

// The CSI sidecar images need to always be the versions set on the chart by default.
yq.set_string_value(
yq.set_value(
YamlKey::try_from(".csi.image.provisionerTag")?,
to_values.csi_provisioner_image_tag(),
upgrade_values_file.path(),
)?;
yq.set_string_value(
yq.set_value(
YamlKey::try_from(".csi.image.attacherTag")?,
to_values.csi_attacher_image_tag(),
upgrade_values_file.path(),
)?;
yq.set_string_value(
yq.set_value(
YamlKey::try_from(".csi.image.snapshotterTag")?,
to_values.csi_snapshotter_image_tag(),
upgrade_values_file.path(),
)?;
yq.set_string_value(
yq.set_value(
YamlKey::try_from(".csi.image.snapshotControllerTag")?,
to_values.csi_snapshot_controller_image_tag(),
upgrade_values_file.path(),
)?;
yq.set_string_value(
yq.set_value(
YamlKey::try_from(".csi.image.registrarTag")?,
to_values.csi_node_driver_registrar_image_tag(),
upgrade_values_file.path(),
Expand Down
52 changes: 35 additions & 17 deletions k8s/upgrade/src/bin/upgrade-job/helm/yaml/yq.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
};
use regex::Regex;
use snafu::{ensure, ResultExt};
use std::{ops::Deref, path::Path, process::Command, str};
use std::{fmt::Display, ops::Deref, path::Path, process::Command, str};

/// This is a container for the String of an input yaml key.
pub(crate) struct YamlKey(String);
Expand Down Expand Up @@ -120,14 +120,14 @@ impl YqV4 {
/// - "beta" - "delta" friend: "ferris"
///
///
/// result:
/// =======
/// foo:
/// bar: "foobar"
/// baz:
/// - "alpha"
/// - "beta"
/// friend: "ferris"
/// result:
/// =======
/// foo:
/// bar: "foobar"
/// baz:
/// - "alpha"
/// - "beta"
/// friend: "ferris"
///
/// Special case: When the default value has changed, and the user has not customised that
/// option, special upgrade values yaml updates have to be added to single out specific cases
Expand Down Expand Up @@ -164,16 +164,34 @@ impl YqV4 {
Ok(yq_merge_output.stdout)
}

/// This sets in-place yaml string values in yaml files.
pub(crate) fn set_string_value(
&self,
key: YamlKey,
value: &str,
filepath: &Path,
) -> Result<()> {
/// This sets in-place yaml values in yaml files.
pub(crate) fn set_value<V>(&self, key: YamlKey, value: V, filepath: &Path) -> Result<()>
where
V: Display + Sized,
{
// Assigning value based on if it needs quotes around it or not.
// Strings require quotes.
let value = match format!("{value}").as_str() {
// Boolean yaml values do not need quotes.
"true" => "true".to_string(),
"false" => "false".to_string(),
// Null doesn't need quotes as well.
"null" => "null".to_string(),
// What remains is integers and strings
something_else => 'other: {
// If it's an integer, then no quotes.
if something_else.parse::<i64>().is_ok() {
break 'other something_else.to_string();
}

// Quotes for a string.
format!(r#""{something_else}""#)
}
};

let yq_set_args = vec_to_strings![
"-i",
format!(r#"{} = "{value}""#, key.as_str()),
format!(r#"{} = {value}"#, key.as_str()),
filepath.to_string_lossy()
];
let yq_set_output =
Expand Down