fix: ConditionEnabled panic issue (#1221)
This commit is contained in:
		
							parent
							
								
									eb21377f39
								
							
						
					
					
						commit
						cb6b91c5dc
					
				|  | @ -2244,8 +2244,16 @@ func markExcludedReleases(releases []ReleaseSpec, selectors []string, commonLabe | ||||||
| 	return filteredReleases, nil | 	return filteredReleases, nil | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | // ConditionEnabled checks if a release condition is enabled based on the provided values.
 | ||||||
|  | // It takes a ReleaseSpec and a map of values as input.
 | ||||||
|  | // If the condition is not specified, it returns true.
 | ||||||
|  | // If the condition is specified but not in the form 'foo.enabled', it returns an error.
 | ||||||
|  | // If the condition is specified and the corresponding value is found in the values map,
 | ||||||
|  | // it checks if the 'enabled' field is set to true. If so, it returns true.
 | ||||||
|  | // Otherwise, it returns false.
 | ||||||
|  | // If the condition is specified but the corresponding value is not found in the values map,
 | ||||||
|  | // it returns an error.
 | ||||||
| func ConditionEnabled(r ReleaseSpec, values map[string]any) (bool, error) { | func ConditionEnabled(r ReleaseSpec, values map[string]any) (bool, error) { | ||||||
| 	var conditionMatch bool |  | ||||||
| 	if len(r.Condition) == 0 { | 	if len(r.Condition) == 0 { | ||||||
| 		return true, nil | 		return true, nil | ||||||
| 	} | 	} | ||||||
|  | @ -2255,16 +2263,22 @@ func ConditionEnabled(r ReleaseSpec, values map[string]any) (bool, error) { | ||||||
| 	} | 	} | ||||||
| 	if v, ok := values[conditionSplit[0]]; ok { | 	if v, ok := values[conditionSplit[0]]; ok { | ||||||
| 		if v == nil { | 		if v == nil { | ||||||
| 			panic(fmt.Sprintf("environment values field '%s' is nil", conditionSplit[0])) | 			return false, fmt.Errorf("environment values field '%s' is nil", conditionSplit[0]) | ||||||
| 		} | 		} | ||||||
| 		if v.(map[string]any)["enabled"] == true { | 		vm, ok := v.(map[string]any) | ||||||
| 			conditionMatch = true | 		if !ok { | ||||||
|  | 			return false, fmt.Errorf("environment values field '%s' is not a map", conditionSplit[0]) | ||||||
| 		} | 		} | ||||||
| 	} else { | 		vv, ok := vm["enabled"] | ||||||
| 		panic(fmt.Sprintf("environment values does not contain field '%s'", conditionSplit[0])) | 		if !ok { | ||||||
|  | 			return false, nil | ||||||
|  | 		} | ||||||
|  | 		if vv == true { | ||||||
|  | 			return true, nil | ||||||
|  | 		} | ||||||
|  | 		return false, nil | ||||||
| 	} | 	} | ||||||
| 
 | 	return false, fmt.Errorf("environment values does not contain field '%s'", conditionSplit[0]) | ||||||
| 	return conditionMatch, nil |  | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func unmarkNeedsAndTransitives(filteredReleases []Release, allReleases []ReleaseSpec) { | func unmarkNeedsAndTransitives(filteredReleases []Release, allReleases []ReleaseSpec) { | ||||||
|  |  | ||||||
|  | @ -2429,7 +2429,6 @@ func TestConditionEnabled(t *testing.T) { | ||||||
| 		values    map[string]any | 		values    map[string]any | ||||||
| 		want      bool | 		want      bool | ||||||
| 		wantErr   bool | 		wantErr   bool | ||||||
| 		wantPanic bool |  | ||||||
| 	}{ | 	}{ | ||||||
| 		{ | 		{ | ||||||
| 			name:      "enabled", | 			name:      "enabled", | ||||||
|  | @ -2451,6 +2450,17 @@ func TestConditionEnabled(t *testing.T) { | ||||||
| 			}, | 			}, | ||||||
| 			want: false, | 			want: false, | ||||||
| 		}, | 		}, | ||||||
|  | 		{ | ||||||
|  | 			name:      "typo in condition", | ||||||
|  | 			condition: "fooo.enabled", | ||||||
|  | 			values: map[string]any{ | ||||||
|  | 				"foo": map[string]any{ | ||||||
|  | 					"enabled": true, | ||||||
|  | 				}, | ||||||
|  | 			}, | ||||||
|  | 			want:    false, | ||||||
|  | 			wantErr: true, | ||||||
|  | 		}, | ||||||
| 		{ | 		{ | ||||||
| 			name:      "missing enabled", | 			name:      "missing enabled", | ||||||
| 			condition: "foo.enabled", | 			condition: "foo.enabled", | ||||||
|  | @ -2467,13 +2477,15 @@ func TestConditionEnabled(t *testing.T) { | ||||||
| 			values: map[string]any{ | 			values: map[string]any{ | ||||||
| 				"foo": nil, | 				"foo": nil, | ||||||
| 			}, | 			}, | ||||||
| 			wantPanic: true, | 			want:    false, | ||||||
|  | 			wantErr: true, | ||||||
| 		}, | 		}, | ||||||
| 		{ | 		{ | ||||||
| 			name:      "foo missing", | 			name:      "foo missing", | ||||||
| 			condition: "foo.enabled", | 			condition: "foo.enabled", | ||||||
| 			values:    map[string]any{}, | 			values:    map[string]any{}, | ||||||
| 			wantPanic: true, | 			want:      false, | ||||||
|  | 			wantErr:   true, | ||||||
| 		}, | 		}, | ||||||
| 		{ | 		{ | ||||||
| 			name:      "wrong suffix", | 			name:      "wrong suffix", | ||||||
|  | @ -2502,13 +2514,6 @@ func TestConditionEnabled(t *testing.T) { | ||||||
| 	for i := range tests { | 	for i := range tests { | ||||||
| 		tt := tests[i] | 		tt := tests[i] | ||||||
| 		t.Run(tt.name, func(t *testing.T) { | 		t.Run(tt.name, func(t *testing.T) { | ||||||
| 			if tt.wantPanic { |  | ||||||
| 				defer func() { |  | ||||||
| 					if r := recover(); r == nil { |  | ||||||
| 						t.Errorf("ConditionEnabled() for %s expected panic", tt.name) |  | ||||||
| 					} |  | ||||||
| 				}() |  | ||||||
| 			} |  | ||||||
| 			res, err := ConditionEnabled(ReleaseSpec{Condition: tt.condition}, tt.values) | 			res, err := ConditionEnabled(ReleaseSpec{Condition: tt.condition}, tt.values) | ||||||
| 			if tt.wantErr { | 			if tt.wantErr { | ||||||
| 				if err == nil { | 				if err == nil { | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue