Merge tag 'v1.14.9' into sync-upstream-release-1.14

Release v1.14.9

go.mod

@@ -184,12 +184,12 @@ require (
 	go.uber.org/atomic v1.11.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	go.uber.org/zap v1.26.0 // indirect
-	golang.org/x/crypto v0.17.0 // indirect
+	golang.org/x/crypto v0.21.0 // indirect
 	golang.org/x/mod v0.13.0 // indirect
-	golang.org/x/net v0.19.0 // indirect // indirect
+	golang.org/x/net v0.23.0
 	golang.org/x/oauth2 v0.11.0 // indirect
-	golang.org/x/sys v0.15.0 // indirect
-	golang.org/x/term v0.15.0
+	golang.org/x/sys v0.18.0 // indirect
+	golang.org/x/term v0.18.0
 	golang.org/x/text v0.14.0 // indirect
 	golang.org/x/time v0.3.0 // indirect
 	golang.org/x/tools v0.14.0 // indirect

go.sum

@@ -553,8 +553,8 @@ golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0
 golang.org/x/crypto v0.3.1-0.20221117191849-2c476679df9a/go.mod h1:hebNnKkNXi2UzZN1eVRvBB7co0a+JxK6XbPiWVs/3J4=
 golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58=
 golang.org/x/crypto v0.7.0/go.mod h1:pYwdfH91IfpZVANVyUOhSIPZaFoJGxTFbZhFTx+dXZU=
-golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k=
-golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4=
+golang.org/x/crypto v0.21.0 h1:X31++rzVUdKhX5sWmSOFZxx8UW/ldWx55cbf08iNAMA=
+golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8=
@@ -636,8 +636,8 @@ golang.org/x/net v0.2.0/go.mod h1:KqCZLdyyvdV855qA2rE3GC2aiw5xGR5TEjj8smXukLY=
 golang.org/x/net v0.5.0/go.mod h1:DivGGAXEgPSlEBzxGzZI+ZLohi+xUj054jfeKui00ws=
 golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
 golang.org/x/net v0.8.0/go.mod h1:QVkue5JL9kW//ek3r6jTKnTFis1tRmNAW2P1shuFdJc=
-golang.org/x/net v0.19.0 h1:zTwKpTd2XuCqf8huc7Fo2iSy+4RHPd10s4KzeTnVr1c=
-golang.org/x/net v0.19.0/go.mod h1:CfAk/cbD4CthTvqiEl8NpboMuiuOYsAr/7NOjZJtv1U=
+golang.org/x/net v0.23.0 h1:7EYJ93RZ9vYSZAIb2x3lnuvqO5zneoD6IvWjuhfxjTs=
+golang.org/x/net v0.23.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
 golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
@@ -720,17 +720,17 @@ golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.4.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc=
-golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4=
+golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
 golang.org/x/term v0.0.0-20220526004731-065cf7ba2467/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
 golang.org/x/term v0.2.0/go.mod h1:TVmDHMZPmdnySmBfhjOoOdhjzdE1h4u1VwSiw2l1Nuc=
 golang.org/x/term v0.4.0/go.mod h1:9P2UbLfCdcvo3p/nzKvsmas4TnlujnuoV9hGgYzW1lQ=
 golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
 golang.org/x/term v0.6.0/go.mod h1:m6U89DPEgQRMq3DNkDClhWw02AUbt2daBVO4cn4Hv9U=
-golang.org/x/term v0.15.0 h1:y/Oo/a/q3IXu26lQgl04j/gjuBDOBlx7X6Om1j2CPW4=
-golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0=
+golang.org/x/term v0.18.0 h1:FcHjZXDMxI8mM3nwhX9HlKop4C0YQvCVCdwYl2wOtE8=
+golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58=
 golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=

internal/controller/rbac/definition/roles.go

@@ -110,8 +110,11 @@ func RenderClusterRoles(d *v1.CompositeResourceDefinition) []rbacv1.ClusterRole
 		Rules: []rbacv1.PolicyRule{
 			{
 				APIGroups: []string{d.Spec.Group},
-				Resources: []string{d.Spec.Names.Plural},
-				Verbs:     verbsEdit,
+				Resources: []string{
+					d.Spec.Names.Plural,
+					d.Spec.Names.Plural + suffixStatus,
+				},
+				Verbs: verbsEdit,
 			},
 		},
 	}
@@ -129,8 +132,11 @@ func RenderClusterRoles(d *v1.CompositeResourceDefinition) []rbacv1.ClusterRole
 		Rules: []rbacv1.PolicyRule{
 			{
 				APIGroups: []string{d.Spec.Group},
-				Resources: []string{d.Spec.Names.Plural},
-				Verbs:     verbsView,
+				Resources: []string{
+					d.Spec.Names.Plural,
+					d.Spec.Names.Plural + suffixStatus,
+				},
+				Verbs: verbsView,
 			},
 		},
 	}
@@ -147,8 +153,11 @@ func RenderClusterRoles(d *v1.CompositeResourceDefinition) []rbacv1.ClusterRole
 		Rules: []rbacv1.PolicyRule{
 			{
 				APIGroups: []string{d.Spec.Group},
-				Resources: []string{d.Spec.Names.Plural},
-				Verbs:     verbsBrowse,
+				Resources: []string{
+					d.Spec.Names.Plural,
+					d.Spec.Names.Plural + suffixStatus,
+				},
+				Verbs: verbsBrowse,
 			},
 		},
 	}
@@ -175,14 +184,20 @@ func RenderClusterRoles(d *v1.CompositeResourceDefinition) []rbacv1.ClusterRole
 
 		edit.Rules = append(edit.Rules, rbacv1.PolicyRule{
 			APIGroups: []string{d.Spec.Group},
-			Resources: []string{d.Spec.ClaimNames.Plural},
-			Verbs:     verbsEdit,
+			Resources: []string{
+				d.Spec.ClaimNames.Plural,
+				d.Spec.ClaimNames.Plural + suffixStatus,
+			},
+			Verbs: verbsEdit,
 		})
 
 		view.Rules = append(view.Rules, rbacv1.PolicyRule{
 			APIGroups: []string{d.Spec.Group},
-			Resources: []string{d.Spec.ClaimNames.Plural},
-			Verbs:     verbsView,
+			Resources: []string{
+				d.Spec.ClaimNames.Plural,
+				d.Spec.ClaimNames.Plural + suffixStatus,
+			},
+			Verbs: verbsView,
 		})
 
 		// The browse role only includes composite resources; not claims.

internal/controller/rbac/definition/roles_test.go

@@ -96,7 +96,7 @@ func TestRenderClusterRoles(t *testing.T) {
 					Rules: []rbacv1.PolicyRule{
 						{
 							APIGroups: []string{group},
-							Resources: []string{pluralXR},
+							Resources: []string{pluralXR, pluralXR + suffixStatus},
 							Verbs:     verbsEdit,
 						},
 					},
@@ -114,7 +114,7 @@ func TestRenderClusterRoles(t *testing.T) {
 					Rules: []rbacv1.PolicyRule{
 						{
 							APIGroups: []string{group},
-							Resources: []string{pluralXR},
+							Resources: []string{pluralXR, pluralXR + suffixStatus},
 							Verbs:     verbsView,
 						},
 					},
@@ -131,7 +131,7 @@ func TestRenderClusterRoles(t *testing.T) {
 					Rules: []rbacv1.PolicyRule{
 						{
 							APIGroups: []string{group},
-							Resources: []string{pluralXR},
+							Resources: []string{pluralXR, pluralXR + suffixStatus},
 							Verbs:     verbsBrowse,
 						},
 					},
@@ -195,12 +195,12 @@ func TestRenderClusterRoles(t *testing.T) {
 					Rules: []rbacv1.PolicyRule{
 						{
 							APIGroups: []string{group},
-							Resources: []string{pluralXR},
+							Resources: []string{pluralXR, pluralXR + suffixStatus},
 							Verbs:     verbsEdit,
 						},
 						{
 							APIGroups: []string{group},
-							Resources: []string{pluralXRC},
+							Resources: []string{pluralXRC, pluralXRC + suffixStatus},
 							Verbs:     verbsEdit,
 						},
 					},
@@ -218,12 +218,12 @@ func TestRenderClusterRoles(t *testing.T) {
 					Rules: []rbacv1.PolicyRule{
 						{
 							APIGroups: []string{group},
-							Resources: []string{pluralXR},
+							Resources: []string{pluralXR, pluralXR + suffixStatus},
 							Verbs:     verbsView,
 						},
 						{
 							APIGroups: []string{group},
-							Resources: []string{pluralXRC},
+							Resources: []string{pluralXRC, pluralXRC + suffixStatus},
 							Verbs:     verbsView,
 						},
 					},
@@ -241,7 +241,7 @@ func TestRenderClusterRoles(t *testing.T) {
 					Rules: []rbacv1.PolicyRule{
 						{
 							APIGroups: []string{group},
-							Resources: []string{pluralXR},
+							Resources: []string{pluralXR, pluralXR + suffixStatus},
 							Verbs:     verbsBrowse,
 						},
 					},

internal/xfn/function_runner.go

@@ -147,7 +147,7 @@ func (r *PackagedFunctionRunner) RunFunction(ctx context.Context, name string, r
 // cost of listing and iterating over FunctionRevisions from cache. The default
 // RevisionHistoryLimit is 1, so for most Functions we'd expect there to be two
 // revisions in the cache (one active, and one previously active).
-func (r *PackagedFunctionRunner) getClientConn(ctx context.Context, name string) (*grpc.ClientConn, error) {
+func (r *PackagedFunctionRunner) getClientConn(ctx context.Context, name string) (*grpc.ClientConn, error) { //nolint:gocyclo // Only slightly over (12).
 	log := r.log.WithValues("function", name)
 
 	l := &pkgv1beta1.FunctionRevisionList{}
@@ -170,12 +170,24 @@ func (r *PackagedFunctionRunner) getClientConn(ctx context.Context, name string)
 		return nil, errors.Errorf(errFmtEmptyEndpoint, active.GetName())
 	}
 
+	// If we have a connection for the up-to-date endpoint, return it.
 	r.connsMx.RLock()
 	conn, ok := r.conns[name]
+	if ok && conn.Target() == active.Status.Endpoint {
+		defer r.connsMx.RUnlock()
+		return conn, nil
+	}
 	r.connsMx.RUnlock()
 
+	// Either we didn't have a connection, or it wasn't up-to-date.
+	r.connsMx.Lock()
+	defer r.connsMx.Unlock()
+
+	// Another Goroutine might have updated the connections between when we
+	// released the read lock and took the write lock, so check again.
+	conn, ok = r.conns[name]
 	if ok {
-		// We have a connection for the up-to-date endpoint. Return it.
+		// We now have a connection for the up-to-date endpoint.
 		if conn.Target() == active.Status.Endpoint {
 			return conn, nil
 		}
@@ -185,6 +197,7 @@ func (r *PackagedFunctionRunner) getClientConn(ctx context.Context, name string)
 		// already closed or in the process of closing.
 		log.Debug("Closing gRPC client connection with stale target", "old-target", conn.Target(), "new-target", active.Status.Endpoint)
 		_ = conn.Close()
+		delete(r.conns, name)
 	}
 
 	// This context is only used for setting up the connection.
@@ -198,9 +211,7 @@ func (r *PackagedFunctionRunner) getClientConn(ctx context.Context, name string)
 		return nil, errors.Wrapf(err, errFmtDialFunction, active.Status.Endpoint, active.GetName())
 	}
 
-	r.connsMx.Lock()
 	r.conns[name] = conn
-	r.connsMx.Unlock()
 
 	log.Debug("Created new gRPC client connection", "target", active.Status.Endpoint)
 	return conn, nil
@@ -235,17 +246,16 @@ func (r *PackagedFunctionRunner) GarbageCollectConnectionsNow(ctx context.Contex
 	// path where no connections need garbage collecting we shouldn't
 	// take it at all.
 
+	// No need to take a write lock or list Functions if there's no work to do.
 	r.connsMx.RLock()
-	connections := make([]string, 0, len(r.conns))
-	for name := range r.conns {
-		connections = append(connections, name)
+	if len(r.conns) == 0 {
+		defer r.connsMx.RUnlock()
+		return 0, nil
 	}
 	r.connsMx.RUnlock()
 
-	// No need to list Functions if there's no work to do.
-	if len(connections) == 0 {
-		return 0, nil
-	}
+	r.connsMx.Lock()
+	defer r.connsMx.Unlock()
 
 	l := &pkgv1beta1.FunctionList{}
 	if err := r.client.List(ctx, l); err != nil {
@@ -257,28 +267,20 @@ func (r *PackagedFunctionRunner) GarbageCollectConnectionsNow(ctx context.Contex
 		functionExists[f.GetName()] = true
 	}
 
-	// Build a list of connections to garbage collect.
-	gc := make([]string, 0)
-	for _, name := range connections {
-		if !functionExists[name] {
-			gc = append(gc, name)
+	// Garbage collect connections.
+	closed := 0
+	for name := range r.conns {
+		if functionExists[name] {
+			continue
 		}
-	}
 
-	// No need to take a write lock if there's no work to do.
-	if len(gc) == 0 {
-		return 0, nil
-	}
-
-	r.log.Debug("Closing gRPC client connections for Functions that are no longer installed", "functions", gc)
-	r.connsMx.Lock()
-	for _, name := range gc {
 		// Close only returns an error is if the connection is already
 		// closed or in the process of closing.
 		_ = r.conns[name].Close()
 		delete(r.conns, name)
+		closed++
+		r.log.Debug("Closed gRPC client connection to Function that is no longer installed", "function", name)
 	}
-	r.connsMx.Unlock()
 
-	return len(gc), nil
+	return closed, nil
 }