From f9db3fe2af92ac7b332c5e03434c1e59af3c6a7b Mon Sep 17 00:00:00 2001 From: pratikmahajan Date: Wed, 17 Aug 2022 13:15:21 -0400 Subject: [PATCH 1/2] fix conditional edge for type always conditional edge has an empty promql when the type of conditional block is `Always` This commit skips the serlialization if promql if the promql condition is empty, which is the case for type `Always` Co-Authored-By: W. Trevor King --- cincinnati/src/conditional_edges.rs | 16 +++++-- cincinnati/src/lib.rs | 60 +++++++++++++++++++++++--- cincinnati/src/plugins/external/web.rs | 4 +- cincinnati/src/plugins/mod.rs | 10 ++--- 4 files changed, 72 insertions(+), 18 deletions(-) diff --git a/cincinnati/src/conditional_edges.rs b/cincinnati/src/conditional_edges.rs index ea022c8ab..7a54549ea 100644 --- a/cincinnati/src/conditional_edges.rs +++ b/cincinnati/src/conditional_edges.rs @@ -34,15 +34,16 @@ pub struct ConditionalUpdateRisk { #[serde(default)] pub struct ClusterCondition { #[serde(rename = "type")] - condition_type: String, - promql: PromQLClusterCondition, + pub condition_type: String, + #[serde(skip_serializing_if = "PromQLClusterCondition::is_empty")] + pub promql: PromQLClusterCondition, } /// Contains the PromQL string #[derive(Debug, Serialize, Deserialize, SmartDefault, Clone, Eq, PartialEq, Hash)] #[serde(default)] -struct PromQLClusterCondition { - promql: String, +pub struct PromQLClusterCondition { + pub promql: String, } impl ConditionalEdge { @@ -51,3 +52,10 @@ impl ConditionalEdge { &mut self.edges } } + +impl PromQLClusterCondition { + /// returns true if there is no PromQL condition to serialize. + pub fn is_empty(&self) -> bool { + self.promql.is_empty() + } +} diff --git a/cincinnati/src/lib.rs b/cincinnati/src/lib.rs index e19cf5a34..b92a0f4b9 100644 --- a/cincinnati/src/lib.rs +++ b/cincinnati/src/lib.rs @@ -818,12 +818,49 @@ pub mod testing { use super::*; use crate::plugins::internal::versioned_graph::VersionedGraph; - pub fn generate_graph(include_conditional_edge: bool) -> Graph { + pub fn generate_graph(include_conditional_edge: bool, include_always_condition: bool) -> Graph { let mut graph = Graph::default(); + if include_conditional_edge { + let mut ce = ConditionalEdge { + edge_regex: ConditionalUpdateEdge { + from: "1[.]0[.].*".to_string(), + to: "2.0.0".to_string(), + }, + edges: vec![ConditionalUpdateEdge { + from: "1.0.0".to_string(), + to: "2.0.0".to_string(), + }], + risks: vec![ConditionalUpdateRisk { + url: "https://bug.example.com/show_bug.cgi?id=example".to_string(), + name: "BrokenUpdates".to_string(), + message: "Updates are broken for this provider".to_string(), + matching_rules: vec![ClusterCondition { + condition_type: "PromQL".to_string(), + promql: PromQLClusterCondition { + promql: "cluster_infrastructure_provider{type=\"CloudProvider\"}" + .to_string(), + }, + }], + }], + }; + if include_always_condition { + ce.risks = vec![ConditionalUpdateRisk { + url: "https://bug.example.com/show_bug.cgi?id=example".to_string(), + name: "AllBrokenUpdates".to_string(), + message: "All Updates are broken".to_string(), + matching_rules: vec![ClusterCondition { + condition_type: "Always".to_string(), + promql: Default::default(), + }], + }] + } + graph.conditional_edges = Some(vec![ce]); + } if !include_conditional_edge { graph.conditional_edges = None; } + let v1 = graph.dag.add_node(Release::Concrete(ConcreteRelease { version: String::from("1.0.0"), payload: String::from("image/1.0.0"), @@ -1160,7 +1197,7 @@ mod tests { #[test] fn serialize_graph() { - let graph = generate_graph(false); + let graph = generate_graph(false, false); assert_eq!( serde_json::to_string(&graph).unwrap(), r#"{"nodes":[{"version":"1.0.0","payload":"image/1.0.0","metadata":{}},{"version":"2.0.0","payload":"image/2.0.0","metadata":{}},{"version":"3.0.0","payload":"image/3.0.0","metadata":{}}],"edges":[[0,1],[1,2],[0,2]]}"# @@ -1169,10 +1206,19 @@ mod tests { #[test] fn serialize_graph_with_conditional_edges() { - let graph = generate_graph(true); + let graph = generate_graph(true, false); + assert_eq!( + serde_json::to_string(&graph).unwrap(), + r#"{"nodes":[{"version":"1.0.0","payload":"image/1.0.0","metadata":{}},{"version":"2.0.0","payload":"image/2.0.0","metadata":{}},{"version":"3.0.0","payload":"image/3.0.0","metadata":{}}],"edges":[[0,1],[1,2],[0,2]],"conditionalEdges":[{"edges":[{"from":"1.0.0","to":"2.0.0"}],"risks":[{"url":"https://bug.example.com/show_bug.cgi?id=example","name":"BrokenUpdates","message":"Updates are broken for this provider","matchingRules":[{"type":"PromQL","promql":{"promql":"cluster_infrastructure_provider{type=\"CloudProvider\"}"}}]}]}]}"# + ); + } + + #[test] + fn serialize_graph_with_conditional_edges_always() { + let graph = generate_graph(true, true); assert_eq!( serde_json::to_string(&graph).unwrap(), - r#"{"nodes":[{"version":"1.0.0","payload":"image/1.0.0","metadata":{}},{"version":"2.0.0","payload":"image/2.0.0","metadata":{}},{"version":"3.0.0","payload":"image/3.0.0","metadata":{}}],"edges":[[0,1],[1,2],[0,2]],"conditionalEdges":[]}"# + r#"{"nodes":[{"version":"1.0.0","payload":"image/1.0.0","metadata":{}},{"version":"2.0.0","payload":"image/2.0.0","metadata":{}},{"version":"3.0.0","payload":"image/3.0.0","metadata":{}}],"edges":[[0,1],[1,2],[0,2]],"conditionalEdges":[{"edges":[{"from":"1.0.0","to":"2.0.0"}],"risks":[{"url":"https://bug.example.com/show_bug.cgi?id=example","name":"AllBrokenUpdates","message":"All Updates are broken","matchingRules":[{"type":"Always"}]}]}]}"# ); } @@ -1226,7 +1272,7 @@ mod tests { #[test] fn test_graph_eq_true_for_equal_graphs() { - assert_eq!(generate_graph(false), generate_graph(false)) + assert_eq!(generate_graph(false, false), generate_graph(false, false)) } #[test] @@ -1314,10 +1360,10 @@ mod tests { #[test] fn roundtrip_conversion_from_graph_via_plugin_interface() { - let graph_plugin_interface: plugins::interface::Graph = generate_graph(false).into(); + let graph_plugin_interface: plugins::interface::Graph = generate_graph(false, false).into(); let graph_native_converted: Graph = graph_plugin_interface.into(); - assert_eq!(generate_graph(false), graph_native_converted); + assert_eq!(generate_graph(false, false), graph_native_converted); } fn get_test_metadata_fn_mut(key_prefix: &str, key_suffix: &str) -> TestMetadata { diff --git a/cincinnati/src/plugins/external/web.rs b/cincinnati/src/plugins/external/web.rs index 3f47772dd..6fbb1325d 100644 --- a/cincinnati/src/plugins/external/web.rs +++ b/cincinnati/src/plugins/external/web.rs @@ -73,7 +73,7 @@ mod tests { }); let input_internal = InternalIO { - graph: generate_graph(false), + graph: generate_graph(false, false), parameters: [("hello".to_string(), "plugin".to_string())] .iter() .cloned() @@ -107,7 +107,7 @@ mod tests { }); let input_internal = InternalIO { - graph: generate_graph(false), + graph: generate_graph(false, false), parameters: [("hello".to_string(), "plugin".to_string())] .iter() .cloned() diff --git a/cincinnati/src/plugins/mod.rs b/cincinnati/src/plugins/mod.rs index 72aca3a64..92562b00f 100644 --- a/cincinnati/src/plugins/mod.rs +++ b/cincinnati/src/plugins/mod.rs @@ -493,7 +493,7 @@ mod tests { #[test] fn convert_roundtrip_internalio_externalio() { - let graph = generate_graph(false); + let graph = generate_graph(false, false); let input_internal = InternalIO { graph, parameters: [("hello".to_string(), "plugin".to_string())] @@ -568,7 +568,7 @@ mod tests { } let initial_internalio = InternalIO { - graph: generate_graph(false), + graph: generate_graph(false, false), parameters: [("hello".to_string(), "plugin".to_string())] .iter() .cloned() @@ -576,7 +576,7 @@ mod tests { }; let expected_internalio = InternalIO { - graph: generate_graph(false), + graph: generate_graph(false, false), parameters: [ ("hello".to_string(), "plugin".to_string()), ("COUNTER".to_string(), "1".to_string()), @@ -613,7 +613,7 @@ mod tests { } let initial_internalio = InternalIO { - graph: generate_graph(false), + graph: generate_graph(false, false), parameters: [("hello".to_string(), "plugin".to_string())] .iter() .cloned() @@ -624,7 +624,7 @@ mod tests { for i in 0..runs { let expected_internalio = InternalIO { - graph: generate_graph(false), + graph: generate_graph(false, false), parameters: [ ("hello".to_string(), "plugin".to_string()), ("COUNTER".to_string(), format!("{}", i + 1)), From 297f59c98539334f3291be1ef9d2dcc25f420c3e Mon Sep 17 00:00:00 2001 From: pratikmahajan Date: Wed, 17 Aug 2022 15:03:01 -0400 Subject: [PATCH 2/2] add location flag for oc binary download add `-L` to follow redirects in case the URL has changed --- dist/Dockerfile.e2e-ubi8/Dockerfile | 2 +- dist/Dockerfile.e2e/Dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dist/Dockerfile.e2e-ubi8/Dockerfile b/dist/Dockerfile.e2e-ubi8/Dockerfile index f7d3190c4..f348f4d50 100644 --- a/dist/Dockerfile.e2e-ubi8/Dockerfile +++ b/dist/Dockerfile.e2e-ubi8/Dockerfile @@ -23,7 +23,7 @@ WORKDIR "${HOME}/cincinnati" # Get oc CLI RUN mkdir -p ${HOME}/bin && \ - curl https://mirror.openshift.com/pub/openshift-v4/clients/ocp/stable/openshift-client-linux.tar.gz 2>/dev/null | tar xzf - -C "${HOME}/bin/" oc + curl -L https://mirror.openshift.com/pub/openshift-v4/clients/ocp/stable/openshift-client-linux.tar.gz 2>/dev/null | tar xzf - -C "${HOME}/bin/" oc ENV PATH="${PATH}:${HOME}/bin" COPY --from=rust_builder /opt/cincinnati/bin/e2e /usr/bin/cincinnati-e2e-test diff --git a/dist/Dockerfile.e2e/Dockerfile b/dist/Dockerfile.e2e/Dockerfile index a505b28dc..b6e632660 100644 --- a/dist/Dockerfile.e2e/Dockerfile +++ b/dist/Dockerfile.e2e/Dockerfile @@ -38,7 +38,7 @@ WORKDIR "${HOME}/cincinnati" # Get oc CLI RUN mkdir -p ${HOME}/bin && \ - curl https://mirror.openshift.com/pub/openshift-v4/clients/ocp/stable/openshift-client-linux.tar.gz 2>/dev/null | tar xzf - -C "${HOME}/bin/" oc + curl -L https://mirror.openshift.com/pub/openshift-v4/clients/ocp/stable/openshift-client-linux.tar.gz 2>/dev/null | tar xzf - -C "${HOME}/bin/" oc ENV PATH="${PATH}:${HOME}/bin" # Install container tools