diff --git a/docs/features/analysis.md b/docs/features/analysis.md index 0c563591ad..980d592c7b 100644 --- a/docs/features/analysis.md +++ b/docs/features/analysis.md @@ -426,8 +426,9 @@ spec: valueFrom: podTemplateHashValue: Latest ``` -Analysis arguments also support valueFrom for reading metadata fields and passing them as arguments to AnalysisTemplate. -An example would be to reference metadata labels like env and region and passing them along to AnalysisTemplate. +Analysis arguments also support valueFrom for reading any Rollout fields and passing them as arguments to AnalysisTemplate. +An example would be to reference metadata labels like env and region and passing them along to AnalysisTemplate, or any field +from the Rollout status ```yaml apiVersion: argoproj.io/v1alpha1 kind: Rollout @@ -457,6 +458,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.labels['region'] + - name: canary-hash + valueFrom: + fieldRef: + fieldPath: status.canary.weights.canary.podTemplateHash ``` ## BlueGreen Pre Promotion Analysis diff --git a/pkg/apis/rollouts/validation/validation.go b/pkg/apis/rollouts/validation/validation.go index 5afb080ddc..da03a1fd95 100644 --- a/pkg/apis/rollouts/validation/validation.go +++ b/pkg/apis/rollouts/validation/validation.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "strconv" + "strings" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -278,7 +279,7 @@ func ValidateRolloutStrategyCanary(rollout *v1alpha1.Rollout, fldPath *field.Pat for _, arg := range analysisRunArgs { if arg.ValueFrom != nil { - if arg.ValueFrom.FieldRef != nil { + if arg.ValueFrom.FieldRef != nil && strings.HasPrefix(arg.ValueFrom.FieldRef.FieldPath, "metadata") { _, err := fieldpath.ExtractFieldPathAsString(rollout, arg.ValueFrom.FieldRef.FieldPath) if err != nil { allErrs = append(allErrs, field.Invalid(stepFldPath.Child("analyses"), analysisRunArgs, InvalidAnalysisArgsMessage)) diff --git a/pkg/apis/rollouts/validation/validation_references.go b/pkg/apis/rollouts/validation/validation_references.go index b02add3d97..1e89764a71 100644 --- a/pkg/apis/rollouts/validation/validation_references.go +++ b/pkg/apis/rollouts/validation/validation_references.go @@ -382,7 +382,8 @@ func buildAnalysisArgs(args []v1alpha1.AnalysisRunArgument, r *v1alpha1.Rollout) }, }, } - return analysisutil.BuildArgumentsForRolloutAnalysisRun(args, &stableRSDummy, &newRSDummy, r) + res, _ := analysisutil.BuildArgumentsForRolloutAnalysisRun(args, &stableRSDummy, &newRSDummy, r) + return res } // validateAnalysisMetrics validates the metrics of an Analysis object diff --git a/rollout/analysis.go b/rollout/analysis.go index e495b07656..1d9a50fdb5 100644 --- a/rollout/analysis.go +++ b/rollout/analysis.go @@ -347,7 +347,11 @@ func (c *rolloutContext) reconcileBackgroundAnalysisRun() (*v1alpha1.AnalysisRun } func (c *rolloutContext) createAnalysisRun(rolloutAnalysis *v1alpha1.RolloutAnalysis, infix string, labels map[string]string) (*v1alpha1.AnalysisRun, error) { - args := analysisutil.BuildArgumentsForRolloutAnalysisRun(rolloutAnalysis.Args, c.stableRS, c.newRS, c.rollout) + args, err := analysisutil.BuildArgumentsForRolloutAnalysisRun(rolloutAnalysis.Args, c.stableRS, c.newRS, c.rollout) + if err != nil { + return nil, err + } + podHash := replicasetutil.GetPodTemplateHash(c.newRS) if podHash == "" { return nil, fmt.Errorf("Latest ReplicaSet '%s' has no pod hash in the labels", c.newRS.Name) diff --git a/rollout/experiment.go b/rollout/experiment.go index 807d44ddb7..eddfc64969 100644 --- a/rollout/experiment.go +++ b/rollout/experiment.go @@ -106,7 +106,11 @@ func GetExperimentFromTemplate(r *v1alpha1.Rollout, stableRS, newRS *appsv1.Repl } for i := range step.Analyses { analysis := step.Analyses[i] - args := analysisutil.BuildArgumentsForRolloutAnalysisRun(analysis.Args, stableRS, newRS, r) + args, err := analysisutil.BuildArgumentsForRolloutAnalysisRun(analysis.Args, stableRS, newRS, r) + if err != nil { + return nil, err + } + analysisTemplate := v1alpha1.ExperimentAnalysisTemplateRef{ Name: analysis.Name, TemplateName: analysis.TemplateName, diff --git a/utils/analysis/factory.go b/utils/analysis/factory.go index 90b1a7de1f..0422daef8d 100644 --- a/utils/analysis/factory.go +++ b/utils/analysis/factory.go @@ -3,17 +3,20 @@ package analysis import ( "encoding/json" "fmt" + "regexp" "strconv" + "strings" + "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" templateutil "github.com/argoproj/argo-rollouts/utils/template" - "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" appsv1 "k8s.io/api/apps/v1" "k8s.io/kubernetes/pkg/fieldpath" ) // BuildArgumentsForRolloutAnalysisRun builds the arguments for a analysis base created by a rollout -func BuildArgumentsForRolloutAnalysisRun(args []v1alpha1.AnalysisRunArgument, stableRS, newRS *appsv1.ReplicaSet, r *v1alpha1.Rollout) []v1alpha1.Argument { +func BuildArgumentsForRolloutAnalysisRun(args []v1alpha1.AnalysisRunArgument, stableRS, newRS *appsv1.ReplicaSet, r *v1alpha1.Rollout) ([]v1alpha1.Argument, error) { + var err error arguments := []v1alpha1.Argument{} for i := range args { arg := args[i] @@ -26,21 +29,28 @@ func BuildArgumentsForRolloutAnalysisRun(args []v1alpha1.AnalysisRunArgument, st case v1alpha1.Stable: value = stableRS.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] } - } else { - if arg.ValueFrom.FieldRef != nil { - value, _ = fieldpath.ExtractFieldPathAsString(r, arg.ValueFrom.FieldRef.FieldPath) + } else if arg.ValueFrom.FieldRef != nil { + if strings.HasPrefix(arg.ValueFrom.FieldRef.FieldPath, "metadata") { + value, err = fieldpath.ExtractFieldPathAsString(r, arg.ValueFrom.FieldRef.FieldPath) + if err != nil { + return nil, err + } + } else { + // in case of error - return empty value for Validation stage, so it will pass validation + // returned error will only be used in Analysis stage + value, err = extractValueFromRollout(r, arg.ValueFrom.FieldRef.FieldPath) } } - } + analysisArg := v1alpha1.Argument{ Name: arg.Name, Value: &value, } arguments = append(arguments, analysisArg) - } - return arguments + + return arguments, err } // PostPromotionLabels returns a map[string]string of common labels for the post promotion analysis @@ -217,3 +227,42 @@ func ValidateMetric(metric v1alpha1.Metric) error { } return nil } + +func extractValueFromRollout(r *v1alpha1.Rollout, path string) (string, error) { + j, _ := json.Marshal(r) + m := interface{}(nil) + json.Unmarshal(j, &m) + sections := regexp.MustCompile("[\\.\\[\\]]+").Split(path, -1) + for _, section := range sections { + if section == "" { + continue // if path ends with a separator char, Split returns an empty last section + } + + if asArray, ok := m.([]interface{}); ok { + if i, err := strconv.Atoi(section); err != nil { + return "", fmt.Errorf("invalid index '%s'", section) + } else if i >= len(asArray) { + return "", fmt.Errorf("index %d out of range", i) + } else { + m = asArray[i] + } + } else if asMap, ok := m.(map[string]interface{}); ok { + m = asMap[section] + } else { + return "", fmt.Errorf("invalid path %s in rollout", path) + } + } + + if m == nil { + return "", fmt.Errorf("invalid path %s in rollout", path) + } + + var isArray, isMap bool + _, isArray = m.([]interface{}) + _, isMap = m.(map[string]interface{}) + if isArray || isMap { + return "", fmt.Errorf("path %s in rollout must terminate in a primitive value", path) + } + + return fmt.Sprintf("%v", m), nil +} diff --git a/utils/analysis/factory_test.go b/utils/analysis/factory_test.go index 12ec2db117..ceece4ac91 100644 --- a/utils/analysis/factory_test.go +++ b/utils/analysis/factory_test.go @@ -4,22 +4,21 @@ import ( "fmt" "testing" - "k8s.io/apimachinery/pkg/util/intstr" - "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" "github.com/argoproj/argo-rollouts/utils/annotations" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/utils/pointer" ) func TestBuildArgumentsForRolloutAnalysisRun(t *testing.T) { - new := v1alpha1.Latest stable := v1alpha1.Stable + annotationPath := fmt.Sprintf("metadata.annotations['%s']", annotations.RevisionAnnotation) rolloutAnalysis := &v1alpha1.RolloutAnalysis{ Args: []v1alpha1.AnalysisRunArgument{ { @@ -50,6 +49,18 @@ func TestBuildArgumentsForRolloutAnalysisRun(t *testing.T) { FieldRef: &v1alpha1.FieldRef{FieldPath: "metadata.labels['env']"}, }, }, + { + Name: annotationPath, + ValueFrom: &v1alpha1.ArgumentValueFrom{ + FieldRef: &v1alpha1.FieldRef{FieldPath: annotationPath}, + }, + }, + { + Name: "status.pauseConditions[0].reason", + ValueFrom: &v1alpha1.ArgumentValueFrom{ + FieldRef: &v1alpha1.FieldRef{FieldPath: "status.pauseConditions[0].reason"}, + }, + }, }, } stableRS := &appsv1.ReplicaSet{ @@ -64,7 +75,6 @@ func TestBuildArgumentsForRolloutAnalysisRun(t *testing.T) { Labels: map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: "123456"}, }, } - ro := &v1alpha1.Rollout{ ObjectMeta: metav1.ObjectMeta{ UID: uuid.NewUUID(), @@ -102,16 +112,24 @@ func TestBuildArgumentsForRolloutAnalysisRun(t *testing.T) { "env": "test", }}, }, - Status: v1alpha1.RolloutStatus{}, + Status: v1alpha1.RolloutStatus{ + PauseConditions: []v1alpha1.PauseCondition{ + { + Reason: "test-reason", + }, + }, + }, } - args := BuildArgumentsForRolloutAnalysisRun(rolloutAnalysis.Args, stableRS, newRS, ro) + args, err := BuildArgumentsForRolloutAnalysisRun(rolloutAnalysis.Args, stableRS, newRS, ro) + assert.NoError(t, err) assert.Contains(t, args, v1alpha1.Argument{Name: "hard-coded-value-key", Value: pointer.StringPtr("hard-coded-value")}) assert.Contains(t, args, v1alpha1.Argument{Name: "stable-key", Value: pointer.StringPtr("abcdef")}) assert.Contains(t, args, v1alpha1.Argument{Name: "new-key", Value: pointer.StringPtr("123456")}) assert.Contains(t, args, v1alpha1.Argument{Name: "metadata.labels['app']", Value: pointer.StringPtr("app")}) assert.Contains(t, args, v1alpha1.Argument{Name: "metadata.labels['env']", Value: pointer.StringPtr("test")}) - + assert.Contains(t, args, v1alpha1.Argument{Name: annotationPath, Value: pointer.StringPtr("1")}) + assert.Contains(t, args, v1alpha1.Argument{Name: "status.pauseConditions[0].reason", Value: pointer.StringPtr("test-reason")}) } func TestPrePromotionLabels(t *testing.T) { @@ -426,3 +444,81 @@ func TestResolveMetricArgsWithQuotes(t *testing.T) { assert.NoError(t, err) assert.Equal(t, fmt.Sprintf(arg), newMetric.SuccessCondition) } + +func Test_extractValueFromRollout(t *testing.T) { + ro := &v1alpha1.Rollout{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Labels: map[string]string{ + "app": "app", + }, + }, + Status: v1alpha1.RolloutStatus{ + PauseConditions: []v1alpha1.PauseCondition{ + { + Reason: "test-reason", + }, + }, + }, + } + tests := map[string]struct { + path string + want string + wantErr string + }{ + "should return a simple metadata value": { + path: "metadata.name", + want: "test", + }, + "should return a label using dot notation": { + path: "metadata.labels.app", + want: "app", + }, + "should fail returning a label using accessor notation": { + path: "metadata.labels['app']", + wantErr: "invalid path metadata.labels['app'] in rollout", + }, + "should return a status value": { + path: "status.pauseConditions[0].reason", + want: "test-reason", + }, + "should fail when array indexer is not an int": { + path: "status.pauseConditions[blah].reason", + wantErr: "invalid index 'blah'", + }, + "should fail when array indexer is out of range": { + path: "status.pauseConditions[12].reason", + wantErr: "index 12 out of range", + }, + "should fail when path references an empty field": { + path: "status.pauseConditions[0].startTime", + wantErr: "invalid path status.pauseConditions[0].startTime in rollout", + }, + "should fail when path is inavlid": { + path: "some.invalid[2].non.existing.path", + wantErr: "invalid path some.invalid[2].non.existing.path in rollout", + }, + "should fail when path references a non-primitive value": { + path: "status.pauseConditions[0]", + wantErr: "path status.pauseConditions[0] in rollout must terminate in a primitive value", + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + got, err := extractValueFromRollout(ro, tt.path) + if err != nil { + if tt.wantErr != "" { + assert.EqualError(t, err, tt.wantErr) + } else { + t.Errorf("extractValueFromRollout() error = %v", err) + } + + return + } + + if got != tt.want { + t.Errorf("extractValueFromRollout() = %v, want %v", got, tt.want) + } + }) + } +}