mirror of https://github.com/linkerd/linkerd2.git
				
				
				
			Allow `Server` CRD to have empty `PodSelector` (#7925)
Fixes #7904 Allow the `Server` CRD to have the `PodSelector` entry be an empty object, by removing the `omitempty` tag from its go type definition and the `oneof` section in the CRD. No update to the CRD version is required, as this is BC change -- The CRD overriding was tested fine. Also added some unit tests to confirm podSelector conditions are ANDed, and some minor refactorings in the `Selector` constructors. Co-authored-by: Oliver Gould <ver@buoyant.io>
This commit is contained in:
		
							parent
							
								
									8fd1a291e4
								
							
						
					
					
						commit
						a268ff11c9
					
				|  | @ -95,9 +95,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -95,9 +95,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -95,9 +95,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -95,9 +95,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -95,9 +95,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -95,9 +95,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -95,9 +95,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -95,9 +95,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -95,9 +95,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -95,9 +95,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -97,9 +97,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -97,9 +97,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -95,9 +95,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -95,9 +95,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -95,9 +95,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -95,9 +95,9 @@ spec: | ||||||
|                   type: object |                   type: object | ||||||
|                   description: >- |                   description: >- | ||||||
|                     Selects pods in the same namespace. |                     Selects pods in the same namespace. | ||||||
|                   oneOf: | 
 | ||||||
|                     - required: [matchExpressions] |                     The result of matchLabels and matchExpressions are ANDed. | ||||||
|                     - required: [matchLabels] |                     Selects all if empty. | ||||||
|                   properties: |                   properties: | ||||||
|                     matchLabels: |                     matchLabels: | ||||||
|                       type: object |                       type: object | ||||||
|  |  | ||||||
|  | @ -29,7 +29,7 @@ type Server struct { | ||||||
| 
 | 
 | ||||||
| // ServerSpec specifies a Server resource.
 | // ServerSpec specifies a Server resource.
 | ||||||
| type ServerSpec struct { | type ServerSpec struct { | ||||||
| 	PodSelector   *metav1.LabelSelector `json:"podSelector,omitempty"` | 	PodSelector   *metav1.LabelSelector `json:"podSelector"` | ||||||
| 	Port          intstr.IntOrString    `json:"port,omitempty"` | 	Port          intstr.IntOrString    `json:"port,omitempty"` | ||||||
| 	ProxyProtocol string                `json:"proxyProtocol,omitempty"` | 	ProxyProtocol string                `json:"proxyProtocol,omitempty"` | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -27,7 +27,8 @@ pub enum Operator { | ||||||
|     DoesNotExist, |     DoesNotExist, | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /// Selects a set of pods that expose a server.
 | /// Selects a set of pods that expose a server. The result of `match_labels` and
 | ||||||
|  | /// `match_expressions` are ANDed.
 | ||||||
| #[derive(Clone, Debug, Eq, PartialEq, Default, Deserialize, Serialize, JsonSchema)] | #[derive(Clone, Debug, Eq, PartialEq, Default, Deserialize, Serialize, JsonSchema)] | ||||||
| #[serde(rename_all = "camelCase")] | #[serde(rename_all = "camelCase")] | ||||||
| pub struct Selector { | pub struct Selector { | ||||||
|  | @ -38,14 +39,22 @@ pub struct Selector { | ||||||
| // === Selector ===
 | // === Selector ===
 | ||||||
| 
 | 
 | ||||||
| impl Selector { | impl Selector { | ||||||
|     pub fn from_expressions(exprs: Expressions) -> Self { |     #[cfg(test)] | ||||||
|  |     fn new(labels: Map, exprs: Expressions) -> Self { | ||||||
|  |         Self { | ||||||
|  |             match_labels: Some(labels), | ||||||
|  |             match_expressions: Some(exprs), | ||||||
|  |         } | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|  |     fn from_expressions(exprs: Expressions) -> Self { | ||||||
|         Self { |         Self { | ||||||
|             match_labels: None, |             match_labels: None, | ||||||
|             match_expressions: Some(exprs), |             match_expressions: Some(exprs), | ||||||
|         } |         } | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     pub fn from_map(map: Map) -> Self { |     fn from_map(map: Map) -> Self { | ||||||
|         Self { |         Self { | ||||||
|             match_labels: Some(map), |             match_labels: Some(map), | ||||||
|             match_expressions: None, |             match_expressions: None, | ||||||
|  | @ -201,6 +210,32 @@ mod tests { | ||||||
|                 true, |                 true, | ||||||
|                 "expression match", |                 "expression match", | ||||||
|             ), |             ), | ||||||
|  |             ( | ||||||
|  |                 Selector::new( | ||||||
|  |                     Map::from([("foo".to_string(), "bar".to_string())]), | ||||||
|  |                     vec![Expression { | ||||||
|  |                         key: "bah".into(), | ||||||
|  |                         operator: Operator::In, | ||||||
|  |                         values: Some(Some("bar".to_string()).into_iter().collect()), | ||||||
|  |                     }], | ||||||
|  |                 ), | ||||||
|  |                 Labels::from_iter(vec![("foo", "bar"), ("bah", "baz")]), | ||||||
|  |                 false, | ||||||
|  |                 "matches labels but not expressions", | ||||||
|  |             ), | ||||||
|  |             ( | ||||||
|  |                 Selector::new( | ||||||
|  |                     Map::from([("foo".to_string(), "bar".to_string())]), | ||||||
|  |                     vec![Expression { | ||||||
|  |                         key: "bah".into(), | ||||||
|  |                         operator: Operator::In, | ||||||
|  |                         values: Some(Some("bar".to_string()).into_iter().collect()), | ||||||
|  |                     }], | ||||||
|  |                 ), | ||||||
|  |                 Labels::from_iter(vec![("foo", "bar"), ("bah", "bar")]), | ||||||
|  |                 true, | ||||||
|  |                 "matches both labels and expressions", | ||||||
|  |             ), | ||||||
|         ] { |         ] { | ||||||
|             assert_eq!(selector.matches(labels), *matches, "{}", msg); |             assert_eq!(selector.matches(labels), *matches, "{}", msg); | ||||||
|         } |         } | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue