Browse Source

Remove unnecessaray alias pass from assets

Signed-off-by: Michael Dresser <michaelmdresser@gmail.com>
Michael Dresser 2 năm trước cách đây
mục cha
commit
566b54073d
3 tập tin đã thay đổi với 3 bổ sung119 xóa
  1. 1 1
      pkg/kubecost/allocationmatcher_test.go
  2. 1 1
      pkg/kubecost/asset.go
  3. 1 117
      pkg/kubecost/assetmatcher.go

+ 1 - 1
pkg/kubecost/allocationmatcher_test.go

@@ -48,7 +48,7 @@ func TestAliasPass(t *testing.T) {
 	}
 
 	for _, c := range cases {
-		pass := NewAliasPass(*labelConfig)
+		pass := NewAllocationAliasPass(*labelConfig)
 
 		t.Run(c.name, func(t *testing.T) {
 			result, err := pass.Exec(c.input)

+ 1 - 1
pkg/kubecost/asset.go

@@ -2748,7 +2748,7 @@ func (as *AssetSet) AggregateBy(aggregateBy []string, opts *AssetAggregationOpti
 	if opts.Filter == nil {
 		filter = &matcher.AllPass[Asset]{}
 	} else {
-		compiler := NewAssetMatchCompiler(opts.LabelConfig)
+		compiler := NewAssetMatchCompiler()
 		var err error
 		filter, err = compiler.Compile(opts.Filter)
 		if err != nil {

+ 1 - 117
pkg/kubecost/assetmatcher.go

@@ -7,9 +7,7 @@ import (
 	afilter "github.com/opencost/opencost/pkg/filter21/asset"
 	"github.com/opencost/opencost/pkg/filter21/ast"
 	"github.com/opencost/opencost/pkg/filter21/matcher"
-	"github.com/opencost/opencost/pkg/filter21/ops"
 	"github.com/opencost/opencost/pkg/filter21/transform"
-	"github.com/opencost/opencost/pkg/log"
 )
 
 // AssetMatcher is a matcher implementation for Asset instances,
@@ -26,14 +24,9 @@ type AssetMatcher matcher.Matcher[Asset]
 // If storage interfaces every support querying natively by alias (e.g. if a
 // data store contained a "product" attribute on an Asset row), that should
 // be handled by a purpose-built AST compiler.
-func NewAssetMatchCompiler(labelConfig *LabelConfig) *matcher.MatchCompiler[Asset] {
+func NewAssetMatchCompiler() *matcher.MatchCompiler[Asset] {
 	passes := []transform.CompilerPass{}
 
-	// The label config pass should be the first pass
-	if labelConfig != nil {
-		passes = append(passes, NewAssetAliasPass(*labelConfig))
-	}
-
 	passes = append(passes,
 		transform.PrometheusKeySanitizePass(),
 		transform.UnallocatedReplacementPass(),
@@ -110,112 +103,3 @@ func assetMapFieldMap(a Asset, identifier ast.Identifier) (map[string]string, er
 	}
 	return nil, fmt.Errorf("Failed to find map[string]string identifier on Asset: %s", identifier.Field.Name)
 }
-
-// assetAPass implements the transform.CompilerPass interface, providing a pass
-// which converts alias nodes to logically-equivalent label/annotation filter
-// nodes based on the label config.
-type assetAliasPass struct {
-	Config              LabelConfig
-	AliasNameToAliasKey map[afilter.AssetAlias]string
-}
-
-// NewAssetAliasPass creates a compiler pass that converts alias nodes to
-// logically-equivalent label/annotation nodes based on the label config.
-func NewAssetAliasPass(config LabelConfig) transform.CompilerPass {
-	aliasNameToAliasKey := map[afilter.AssetAlias]string{
-		// TODO: is external right?
-		afilter.DepartmentProp:  config.DepartmentExternalLabel,
-		afilter.EnvironmentProp: config.EnvironmentExternalLabel,
-		afilter.OwnerProp:       config.OwnerExternalLabel,
-		afilter.ProductProp:     config.ProductExternalLabel,
-		afilter.TeamProp:        config.TeamExternalLabel,
-	}
-
-	return &assetAliasPass{
-		Config:              config,
-		AliasNameToAliasKey: aliasNameToAliasKey,
-	}
-}
-
-// Exec implements the transform.CompilerPass interface for an alias pass.
-// See aliasPass struct documentation for an explanation.
-func (p *assetAliasPass) Exec(filter ast.FilterNode) (ast.FilterNode, error) {
-	if p.AliasNameToAliasKey == nil {
-		return nil, fmt.Errorf("cannot perform alias conversion with nil mapping of alias name -> key")
-	}
-
-	var transformErr error
-	leafTransformerFunc := func(node ast.FilterNode) ast.FilterNode {
-		if transformErr != nil {
-			return node
-		}
-
-		var field *ast.Field
-		var filterValue string
-		var filterOp ast.FilterOp
-
-		switch concrete := node.(type) {
-		// These ops are not alias ops, alias ops can only be base-level ops
-		// like =, !=, etc. No modification required here.
-		case *ast.AndOp, *ast.OrOp, *ast.NotOp, *ast.VoidOp, *ast.ContradictionOp:
-			return node
-
-		case *ast.EqualOp:
-			field = concrete.Left.Field
-			filterValue = concrete.Right
-			filterOp = ast.FilterOpEquals
-		case *ast.ContainsOp:
-			field = concrete.Left.Field
-			filterValue = concrete.Right
-			filterOp = ast.FilterOpContains
-		case *ast.ContainsPrefixOp:
-			field = concrete.Left.Field
-			filterValue = concrete.Right
-			filterOp = ast.FilterOpContainsPrefix
-		case *ast.ContainsSuffixOp:
-			field = concrete.Left.Field
-			filterValue = concrete.Right
-			filterOp = ast.FilterOpContainsSuffix
-		default:
-			transformErr = fmt.Errorf("unknown op '%s' during alias pass", concrete.Op())
-			return node
-		}
-		if field == nil {
-			return node
-		}
-		if !field.IsAlias() {
-			return node
-		}
-
-		filterFieldAlias := afilter.AssetAlias(field.Name)
-		parserAliasKey, ok := p.AliasNameToAliasKey[filterFieldAlias]
-		if !ok {
-			transformErr = fmt.Errorf("unknown alias field '%s'", filterFieldAlias)
-			return node
-		}
-		labelKey := ops.WithKey(afilter.FieldLabel, parserAliasKey)
-
-		switch filterOp {
-		case ast.FilterOpEquals:
-			return ops.Eq(labelKey, filterValue)
-		case ast.FilterOpContains:
-			return ops.Contains(labelKey, filterValue)
-		case ast.FilterOpContainsPrefix:
-			return ops.ContainsPrefix(labelKey, filterValue)
-		case ast.FilterOpContainsSuffix:
-			return ops.ContainsSuffix(labelKey, filterValue)
-		default:
-			transformErr = fmt.Errorf("unexpected failed case match during Asset alias translation, filterOp %s", filterOp)
-			log.Errorf("Unexpected failed case match for Asset alias translation, filterOp %s", filterOp)
-			return node
-		}
-	}
-
-	newFilter := ast.TransformLeaves(filter, leafTransformerFunc)
-
-	if transformErr != nil {
-		return nil, fmt.Errorf("alias pass transform: %w", transformErr)
-	}
-
-	return newFilter, nil
-}