Skip to content

Commit

Permalink
Fixed mirrord verify-config output (metalbear-co#2081)
Browse files Browse the repository at this point in the history
* Changed the way 'targetless' is serialized

* Changelog entry
  • Loading branch information
Razz4780 authored Nov 23, 2023
1 parent 78ba20a commit 0c6adaf
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 9 deletions.
1 change: 1 addition & 0 deletions changelog.d/+targetless-verify-config.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Changed the way `targetless` is printed in `mirrord verify-config` to allow the IDEs to properly show target selection dialogs.
2 changes: 1 addition & 1 deletion mirrord/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ fn main() -> miette::Result<()> {
}
Commands::InternalProxy => internal_proxy::proxy(watch).await?,
Commands::Waitlist(args) => register_to_waitlist(args.email).await?,
Commands::VerifyConfig(config_path) => verify_config(config_path).await?,
Commands::VerifyConfig(args) => verify_config(args).await?,
Commands::Completions(args) => {
let mut cmd: clap::Command = Cli::command();
generate(args.shell, &mut cmd, "mirrord", &mut std::io::stdout());
Expand Down
62 changes: 55 additions & 7 deletions mirrord/cli/src/verify_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,70 @@
use error::Result;
use mirrord_config::{
config::{ConfigContext, MirrordConfig},
target::TargetConfig,
target::{DeploymentTarget, PodTarget, RolloutTarget, Target, TargetConfig},
};
use serde::{Deserialize, Serialize};
use serde::Serialize;

use crate::{config::VerifyConfigArgs, error, LayerFileConfig};

/// Practically the same as [`Target`], but differs in the way the `targetless` option is
/// serialized. [`Target::Targetless`] serializes as `null`, [`VerifiedTarget::Targetless`]
/// serializes as string `"targetless"`. This difference allows the IDEs to correctly decide whether
/// to show the target selection dialog.
///
/// Changing the way [`Target::Targetless`] serializes would be cumbersome for two reasons:
/// 1. It's used in a lot of places, e.g. CRDs
/// 2. `schemars` crate does not support nested `[serde(untagged)]` tags
#[derive(Serialize)]
enum VerifiedTarget {
#[serde(rename = "targetless")]
Targetless,
#[serde(untagged)]
Pod(PodTarget),
#[serde(untagged)]
Deployment(DeploymentTarget),
#[serde(untagged)]
Rollout(RolloutTarget),
}

impl From<Target> for VerifiedTarget {
fn from(value: Target) -> Self {
match value {
Target::Deployment(d) => Self::Deployment(d),
Target::Pod(p) => Self::Pod(p),
Target::Rollout(r) => Self::Rollout(r),
Target::Targetless => Self::Targetless,
}
}
}

#[derive(Serialize)]
struct VerifiedTargetConfig {
path: Option<VerifiedTarget>,
namespace: Option<String>,
}

impl From<TargetConfig> for VerifiedTargetConfig {
fn from(value: TargetConfig) -> Self {
Self {
path: value.path.map(Into::into),
namespace: value.namespace,
}
}
}

/// Produced by calling `verify_config`.
///
/// It's consumed by the IDEs to check if a config is valid, or missing something, without starting
/// mirrord fully.
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Serialize)]
#[serde(tag = "type")]
enum VerifiedConfig {
/// mirrord is able to run with this config, but it might have some issues or weird behavior
/// depending on the `warnings`.
Success {
/// A valid, verified config for the `target` part of mirrord.
config: TargetConfig,
config: VerifiedTargetConfig,
/// Improper combination of features was requested, but mirrord can still run.
warnings: Vec<String>,
},
Expand Down Expand Up @@ -69,14 +115,16 @@ enum VerifiedConfig {
pub(super) async fn verify_config(VerifyConfigArgs { ide, path }: VerifyConfigArgs) -> Result<()> {
let mut config_context = ConfigContext::new(ide);

let verified = match LayerFileConfig::from_path(path)
let layer_config = LayerFileConfig::from_path(path)
.and_then(|config| config.generate_config(&mut config_context))
.and_then(|config| {
config.verify(&mut config_context)?;
Ok(config)
}) {
});

let verified = match layer_config {
Ok(config) => VerifiedConfig::Success {
config: config.target,
config: config.target.into(),
warnings: config_context.get_warnings().to_owned(),
},
Err(fail) => VerifiedConfig::Fail {
Expand Down
2 changes: 1 addition & 1 deletion mirrord/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ impl LayerFileConfig {
match path.as_ref().extension().and_then(|os_val| os_val.to_str()) {
Some("json") => Ok(serde_json::from_str::<Self>(&rendered)?),
Some("toml") => Ok(toml::from_str::<Self>(&rendered)?),
Some("yaml") => Ok(serde_yaml::from_str::<Self>(&rendered)?),
Some("yaml" | "yml") => Ok(serde_yaml::from_str::<Self>(&rendered)?),
_ => Err(ConfigError::UnsupportedFormat),
}
}
Expand Down
1 change: 1 addition & 0 deletions mirrord/config/src/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ impl Target {
}
}
}

/// <!--${internal}-->
/// Mirror the pod specified by [`PodTarget::pod`].
#[derive(Serialize, Deserialize, Clone, Eq, PartialEq, Hash, Debug, JsonSchema)]
Expand Down

0 comments on commit 0c6adaf

Please sign in to comment.