Skip to content
This repository has been archived by the owner on Nov 18, 2021. It is now read-only.

Commit

Permalink
tools/trim: fix list element removal bug
Browse files Browse the repository at this point in the history
This breaks comprehension removal, but that beats
having bugs that break semantics.

Trim has seen its best time since all the changes to
the evaluator and should probably be rewritten come
the new evaluator.

Change-Id: I13b0aa58d3f2c1e4eb0fc617811be7d6078e6265
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/6641
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
  • Loading branch information
mpvl committed Jul 22, 2020
1 parent bdb45b3 commit dd312be
Show file tree
Hide file tree
Showing 29 changed files with 168 additions and 47 deletions.
3 changes: 3 additions & 0 deletions cue/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,9 @@ func (v Value) Len() Value {

// Elem returns the value of undefined element types of lists and structs.
func (v Value) Elem() (Value, bool) {
if v.v == nil {
return Value{}, false
}
ctx := v.ctx()
switch x := v.v.Value.(type) {
case *structLit:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package kube

service: bartender: spec: ports: [{
port: 7080
targetPort: 7080
}]
deployment: bartender: spec: template: spec: containers: [{
image: "gcr.io/myproj/bartender:v0.1.34"
args: [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package kube

service: breaddispatcher: spec: ports: [{
port: 7080
targetPort: 7080
}]
deployment: breaddispatcher: spec: template: spec: containers: [{
image: "gcr.io/myproj/breaddispatcher:v0.3.24"
args: [
Expand Down
4 changes: 4 additions & 0 deletions doc/tutorial/kubernetes/quick/services/frontend/host/kube.cue
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package kube

service: host: spec: ports: [{
port: 7080
targetPort: 7080
}]
deployment: host: spec: {
replicas: 2
template: spec: containers: [{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package kube

service: maitred: spec: ports: [{
port: 7080
targetPort: 7080
}]
deployment: maitred: spec: template: spec: containers: [{
image: "gcr.io/myproj/maitred:v0.0.4"
args: [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package kube

service: valeter: spec: ports: [{
name: "http"
port: 8080
targetPort: 8080
name: "http"
}]
deployment: valeter: spec: template: spec: containers: [{
image: "gcr.io/myproj/valeter:v0.0.4"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package kube

service: waiter: spec: ports: [{
port: 7080
targetPort: 7080
}]
deployment: waiter: spec: {
replicas: 5
template: spec: containers: [{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package kube

service: waterdispatcher: spec: ports: [{
name: "http"
port: 7080
targetPort: 7080
name: "http"
}]
deployment: waterdispatcher: spec: template: spec: containers: [{
image: "gcr.io/myproj/waterdispatcher:v0.0.48"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package kube

service: download: spec: ports: [{
port: 7080
targetPort: 7080
}]
deployment: download: spec: template: spec: containers: [{
image: "gcr.io/myproj/download:v0.0.2"
ports: [{
Expand Down
6 changes: 5 additions & 1 deletion doc/tutorial/kubernetes/quick/services/infra/etcd/kube.cue
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@ package kube
service: etcd: spec: {
clusterIP: "None"
ports: [{
port: 2379
targetPort: 2379
}, {
name: "peer"
port: 2380
targetPort: 2380
name: "peer"
}]
}
statefulSet: etcd: spec: {
Expand Down
4 changes: 3 additions & 1 deletion doc/tutorial/kubernetes/quick/services/infra/events/kube.cue
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package kube

service: events: spec: ports: [{
name: "grpc"
port: 7788
targetPort: 7788
name: "grpc"
}]
deployment: events: spec: {
replicas: 2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ service: tasks: spec: {
type: "LoadBalancer"
loadBalancerIP: "1.2.3.4" // static ip
ports: [{
port: 443
name: "http"
port: 443
targetPort: 7443
name: "http"
}]
}
4 changes: 4 additions & 0 deletions doc/tutorial/kubernetes/quick/services/infra/updater/kube.cue
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package kube

service: updater: spec: ports: [{
port: 8080
targetPort: 8080
}]
deployment: updater: spec: template: spec: {
volumes: [{
name: "secret-updater"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ service: watcher: spec: {
type: "LoadBalancer"
loadBalancerIP: "1.2.3.4." // static ip
ports: [{
name: "http"
port: 7788
targetPort: 7788
name: "http"
}]
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package kube

service: caller: spec: ports: [{
port: 8080
targetPort: 8080
}]
deployment: caller: spec: {
replicas: 3
template: spec: {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package kube

service: dishwasher: spec: ports: [{
port: 8080
targetPort: 8080
}]
deployment: dishwasher: spec: {
replicas: 5
template: spec: {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package kube

service: expiditer: spec: ports: [{
port: 8080
targetPort: 8080
}]
deployment: expiditer: spec: template: spec: containers: [{
image: "gcr.io/myproj/expiditer:v0.5.34"
args: [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package kube

service: headchef: spec: ports: [{
port: 8080
targetPort: 8080
}]
deployment: headchef: spec: template: spec: containers: [{
image: "gcr.io/myproj/headchef:v0.2.16"
volumeMounts: [{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package kube

service: linecook: spec: ports: [{
port: 8080
targetPort: 8080
}]
deployment: linecook: spec: template: spec: {
volumes: [{
}, {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package kube

service: pastrychef: spec: ports: [{
port: 8080
targetPort: 8080
}]
deployment: pastrychef: spec: template: spec: {
volumes: [{
}, {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package kube

service: souschef: spec: ports: [{
port: 8080
targetPort: 8080
}]
deployment: souschef: spec: template: spec: containers: [{
image: "gcr.io/myproj/souschef:v0.5.3"
}]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ service: alertmanager: {
spec: {
// type: ClusterIP
ports: [{
name: "main"
name: "main"
port: 9093
targetPort: 9093
}]
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ service: "node-exporter": {
clusterIP: "None"
ports: [{
name: "metrics"
port: 9100
}]
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ service: prometheus: {
type: "NodePort"
ports: [{
name: "main"
port: 9090
nodePort: 30900
}]
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@
package kube

service: authproxy: spec: ports: [{
port: 4180
targetPort: 4180
}]
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ service: goget: spec: {
type: "LoadBalancer"
loadBalancerIP: "1.3.5.7" // static ip
ports: [{
port: 443
name: "https"
port: 443
targetPort: 7443
name: "https"
}]
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@ service: nginx: spec: {
type: "LoadBalancer"
loadBalancerIP: "1.3.4.5"
ports: [{
name: "http"
port: 80 // the port that this service should serve on
// the container on each pod to connect to, can be a name
// (e.g. 'www') or a number (e.g. 80)
targetPort: 80
name: "http"
}, {
port: 443
name: "https"
}]
}
86 changes: 52 additions & 34 deletions tools/trim/trim.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,26 +267,30 @@ func (t *trimSet) trim(label string, v, m, scope cue.Value) (rmSet []ast.Node) {
// Identify generated components and unify them with the mixin value.
exists := false
for _, v := range vSplit {
if src := v.Source(); t.alwaysGen[src] || inNodes(gen, src) {
if !v.IsConcrete() {
// The template has an expression that cannot be fully
// resolved. Attempt to complete the expression by
// evaluting it within the struct to which the template
// is applied.
expr := internal.ToExpr(v.Syntax())

// TODO: this only resolves references contained in scope.
v = internal.EvalExpr(scope, expr).(cue.Value)
}
src := v.Source()
alwaysGen := t.alwaysGen[src]
inNodes := inNodes(gen, src)
if !(alwaysGen || inNodes) {
continue
}
if !v.IsConcrete() {
// The template has an expression that cannot be fully
// resolved. Attempt to complete the expression by
// evaluting it within the struct to which the template
// is applied.
expr := internal.ToExpr(v.Syntax())

// TODO: this only resolves references contained in scope.
v = internal.EvalExpr(scope, expr).(cue.Value)
}

if w := in.Unify(v); w.Err() == nil {
in = w
}
// One of the sources of this struct is generated. That means
// we can safely delete a non-generated version.
exists = true
gen = append(gen, src)
if w := in.Unify(v); w.Err() == nil {
in = w
}
// One of the sources of this struct is generated. That means
// we can safely delete a non-generated version.
exists = true
gen = append(gen, src)
}

switch v.Kind() {
Expand Down Expand Up @@ -371,39 +375,53 @@ func (t *trimSet) trim(label string, v, m, scope cue.Value) (rmSet []ast.Node) {

case cue.ListKind:
mIter, _ := m.List()
elem, hasElem := m.Elem()
i := 0
rmElem := []ast.Node{}
allowRemove := true
for iter, _ := v.List(); iter.Next(); i++ {
mIter.Next()
rm := t.trim(strconv.Itoa(i), iter.Value(), mIter.Value(), scope)
var m cue.Value
if mIter.Next() {
m = mIter.Value()
} else if hasElem {
m = elem
allowRemove = false
} else {
allowRemove = false
break
}
rm := t.trim(strconv.Itoa(i), iter.Value(), m, scope)
rmElem = append(rmElem, rm...)
}

// Signal the removal of lists of which all elements have been marked
// for removal.
for _, v := range vSplit {
if src := v.Source(); !t.alwaysGen[src] {
l, ok := src.(*ast.ListLit)
if !ok {
break
}
rmList := true
iter, _ := v.List()
for i := 0; i < len(l.Elts) && iter.Next(); i++ {
if !inNodes(rmElem, l.Elts[i]) {
rmList = false
if allowRemove {
for _, v := range vSplit {
if src := v.Source(); !t.alwaysGen[src] {
l, ok := src.(*ast.ListLit)
if !ok {
break
}
}
if rmList && m.Exists() && t.canRemove(src) && !inNodes(gen, src) {
rmSet = append(rmSet, src)
rmList := true
iter, _ := v.List()
for i := 0; i < len(l.Elts) && iter.Next(); i++ {
if !inNodes(rmElem, l.Elts[i]) {
rmList = false
break
}
}
if rmList && m.Exists() && t.canRemove(src) && !inNodes(gen, src) {
rmSet = append(rmSet, src)
}
}
}
}
fallthrough

default:
// Mark any subsumed part that is covered by generated config.
in, _ := in.Default()
if in.Err() == nil && v.Subsume(in) == nil {
for _, v := range vSplit {
src := v.Source()
Expand Down

0 comments on commit dd312be

Please sign in to comment.