Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compare .Kind() instead of direct equality checks on a dyn.Value #1520

Merged
merged 8 commits into from
Jun 27, 2024
4 changes: 2 additions & 2 deletions bundle/config/mutator/environments_compat.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ func (m *environmentsToTargets) Apply(ctx context.Context, b *bundle.Bundle) dia
targets := v.Get("targets")

// Return an error if both "environments" and "targets" are set.
if environments != dyn.InvalidValue && targets != dyn.InvalidValue {
if environments.Kind() != dyn.KindInvalid && targets.Kind() != dyn.KindInvalid {
return dyn.InvalidValue, fmt.Errorf(
"both 'environments' and 'targets' are specified; only 'targets' should be used: %s",
environments.Location().String(),
)
}

// Rewrite "environments" to "targets".
if environments != dyn.InvalidValue && targets == dyn.InvalidValue {
if environments.Kind() != dyn.KindInvalid && targets.Kind() == dyn.KindInvalid {
nv, err := dyn.Set(v, "targets", environments)
if err != nil {
return dyn.InvalidValue, err
Expand Down
2 changes: 1 addition & 1 deletion bundle/config/mutator/merge_job_clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (m *mergeJobClusters) jobClusterKey(v dyn.Value) string {

func (m *mergeJobClusters) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
if v == dyn.NilValue {
if v.Kind() == dyn.KindNil {
return v, nil
}

Expand Down
2 changes: 1 addition & 1 deletion bundle/config/mutator/merge_job_tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (m *mergeJobTasks) taskKeyString(v dyn.Value) string {

func (m *mergeJobTasks) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
if v == dyn.NilValue {
if v.Kind() == dyn.KindNil {
return v, nil
}

Expand Down
2 changes: 1 addition & 1 deletion bundle/config/mutator/merge_pipeline_clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (m *mergePipelineClusters) clusterLabel(v dyn.Value) string {

func (m *mergePipelineClusters) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
if v == dyn.NilValue {
if v.Kind() == dyn.KindNil {
return v, nil
}

Expand Down
14 changes: 8 additions & 6 deletions bundle/config/mutator/run_as.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,17 @@ func (e errBothSpAndUserSpecified) Error() string {
}

func validateRunAs(b *bundle.Bundle) error {
runAs := b.Config.RunAs

// Error if neither service_principal_name nor user_name are specified
if runAs.ServicePrincipalName == "" && runAs.UserName == "" {
// Error if neither service_principal_name nor user_name are specified, but the
// run_as section is present.
// TODO: Track location for nil values. This is likely to be a separate PR.
// TODO: Change comparision from directly comparing the dyn.Values to
// checking the kind instead.``
shreyas-goenka marked this conversation as resolved.
Show resolved Hide resolved
if b.Config.Value().Get("run_as").Kind() == dyn.KindNil {
shreyas-goenka marked this conversation as resolved.
Show resolved Hide resolved
return fmt.Errorf("run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified at %s", b.Config.GetLocation("run_as"))
}

// Error if both service_principal_name and user_name are specified
runAs := b.Config.RunAs
if runAs.UserName != "" && runAs.ServicePrincipalName != "" {
return errBothSpAndUserSpecified{
spName: runAs.ServicePrincipalName,
Expand Down Expand Up @@ -163,8 +166,7 @@ func setPipelineOwnersToRunAsIdentity(b *bundle.Bundle) {

func (m *setRunAs) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics {
// Mutator is a no-op if run_as is not specified in the bundle
runAs := b.Config.RunAs
if runAs == nil {
if b.Config.Value().Get("run_as").Kind() == dyn.KindInvalid {
return nil
}

Expand Down
18 changes: 9 additions & 9 deletions bundle/config/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,39 +341,39 @@ func (r *Root) MergeTargetOverrides(name string) error {
}

// Merge `run_as`. This field must be overwritten if set, not merged.
if v := target.Get("run_as"); v != dyn.InvalidValue {
if v := target.Get("run_as"); v.Kind() != dyn.KindInvalid {
root, err = dyn.Set(root, "run_as", v)
if err != nil {
return err
}
}

// Below, we're setting fields on the bundle key, so make sure it exists.
if root.Get("bundle") == dyn.InvalidValue {
if root.Get("bundle").Kind() == dyn.KindInvalid {
root, err = dyn.Set(root, "bundle", dyn.NewValue(map[string]dyn.Value{}, dyn.Location{}))
if err != nil {
return err
}
}

// Merge `mode`. This field must be overwritten if set, not merged.
if v := target.Get("mode"); v != dyn.InvalidValue {
if v := target.Get("mode"); v.Kind() != dyn.KindInvalid {
root, err = dyn.SetByPath(root, dyn.NewPath(dyn.Key("bundle"), dyn.Key("mode")), v)
if err != nil {
return err
}
}

// Merge `compute_id`. This field must be overwritten if set, not merged.
if v := target.Get("compute_id"); v != dyn.InvalidValue {
if v := target.Get("compute_id"); v.Kind() != dyn.KindInvalid {
root, err = dyn.SetByPath(root, dyn.NewPath(dyn.Key("bundle"), dyn.Key("compute_id")), v)
if err != nil {
return err
}
}

// Merge `git`.
if v := target.Get("git"); v != dyn.InvalidValue {
if v := target.Get("git"); v.Kind() != dyn.KindInvalid {
ref, err := dyn.GetByPath(root, dyn.NewPath(dyn.Key("bundle"), dyn.Key("git")))
if err != nil {
ref = dyn.NewValue(map[string]dyn.Value{}, dyn.Location{})
Expand All @@ -386,7 +386,7 @@ func (r *Root) MergeTargetOverrides(name string) error {
}

// If the branch was overridden, we need to clear the inferred flag.
if branch := v.Get("branch"); branch != dyn.InvalidValue {
if branch := v.Get("branch"); branch.Kind() != dyn.KindInvalid {
out, err = dyn.SetByPath(out, dyn.NewPath(dyn.Key("inferred")), dyn.NewValue(false, dyn.Location{}))
if err != nil {
return err
Expand Down Expand Up @@ -414,7 +414,7 @@ func rewriteShorthands(v dyn.Value) (dyn.Value, error) {
// For each target, rewrite the variables block.
return dyn.Map(v, "targets", dyn.Foreach(func(_ dyn.Path, target dyn.Value) (dyn.Value, error) {
// Confirm it has a variables block.
if target.Get("variables") == dyn.InvalidValue {
if target.Get("variables").Kind() == dyn.KindInvalid {
return target, nil
}

Expand Down Expand Up @@ -444,15 +444,15 @@ func validateVariableOverrides(root, target dyn.Value) (err error) {
var tv map[string]variable.Variable

// Collect variables from the root.
if v := root.Get("variables"); v != dyn.InvalidValue {
if v := root.Get("variables"); v.Kind() != dyn.KindInvalid {
err = convert.ToTyped(&rv, v)
if err != nil {
return fmt.Errorf("unable to collect variables from root: %w", err)
}
}

// Collect variables from the target.
if v := target.Get("variables"); v != dyn.InvalidValue {
if v := target.Get("variables"); v.Kind() != dyn.KindInvalid {
err = convert.ToTyped(&tv, v)
if err != nil {
return fmt.Errorf("unable to collect variables from target: %w", err)
Expand Down
4 changes: 2 additions & 2 deletions libs/dyn/convert/from_typed.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func fromTypedStruct(src reflect.Value, ref dyn.Value, options ...fromTypedOptio
}

// Either if the key was set in the reference or the field is not zero-valued, we include it.
if ok || nv != dyn.NilValue {
if ok || nv.Kind() != dyn.KindNil {
out.Set(refk, nv)
}
}
Expand Down Expand Up @@ -186,7 +186,7 @@ func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
refv := ref.Index(i)

// Use nil reference if there is no reference for this index.
if refv == dyn.InvalidValue {
if refv.Kind() == dyn.KindInvalid {
refv = dyn.NilValue
}

Expand Down
2 changes: 1 addition & 1 deletion libs/dyn/convert/to_typed.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func ToTyped(dst any, src dyn.Value) error {
for dstv.Kind() == reflect.Pointer {
// If the source value is nil and the destination is a settable pointer,
// set the destination to nil. Also see `end_to_end_test.go`.
if dstv.CanSet() && src == dyn.NilValue {
if dstv.CanSet() && src.Kind() == dyn.KindNil {
dstv.SetZero()
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion libs/dyn/merge/override.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func Override(leftRoot dyn.Value, rightRoot dyn.Value, visitor OverrideVisitor)
}

func override(basePath dyn.Path, left dyn.Value, right dyn.Value, visitor OverrideVisitor) (dyn.Value, error) {
if left == dyn.NilValue && right == dyn.NilValue {
if left.Kind() == dyn.KindNil && right.Kind() == dyn.KindNil {
return dyn.NilValue, nil
}

Expand Down
Loading