Merge pull request #130 from phisco/sync-upstream-release-1.15

Sync upstream release 1.15
upbound · May 29, 2024 · a9ba61d · a9ba61d
2 parents 57be608 + 9644d86
commit a9ba61d
Show file tree

Hide file tree

Showing 19 changed files with 87 additions and 62 deletions.
diff --git a/cluster/charts/crossplane/templates/rbac-manager-deployment.yaml b/cluster/charts/crossplane/templates/rbac-manager-deployment.yaml
@@ -64,11 +64,13 @@ spec:
               resourceFieldRef:
                 containerName: {{ .Chart.Name }}-init
                 resource: limits.cpu
+                divisor: "1"
           - name: GOMEMLIMIT
             valueFrom:
               resourceFieldRef:
                 containerName: {{ .Chart.Name }}-init
                 resource: limits.memory
+                divisor: "1"
       containers:
       - image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default (printf "v%s" .Chart.AppVersion) }}"
         args:
@@ -97,11 +99,13 @@ spec:
               resourceFieldRef:
                 containerName: {{ .Chart.Name }}
                 resource: limits.cpu
+                divisor: "1"
           - name: GOMEMLIMIT
             valueFrom:
               resourceFieldRef:
                 containerName: {{ .Chart.Name }}
                 resource: limits.memory
+                divisor: "1"
           - name: LEADER_ELECTION
             value: "{{ .Values.rbacManager.leaderElection }}"
         {{- range $key, $value := .Values.extraEnvVarsRBACManager }}

diff --git a/cluster/images/crossplane/Dockerfile b/cluster/images/crossplane/Dockerfile
@@ -1,4 +1,4 @@
-FROM gcr.io/distroless/static@sha256:9be3fcc6abeaf985b5ecce59451acbcbb15e7be39472320c538d0d55a0834edc
+FROM gcr.io/distroless/static@sha256:41972110a1c1a5c0b6adb283e8aa092c43c31f7c5d79b8656fbffff2c3e61f05
 
 ARG TARGETOS
 ARG TARGETARCH

diff --git a/cmd/crank/beta/validate/manager.go b/cmd/crank/beta/validate/manager.go
@@ -192,13 +192,17 @@ func (m *Manager) addDependencies() error {
 		deps := cfg.Spec.MetaSpec.DependsOn
 		for _, dep := range deps {
 			image := ""
-			if dep.Configuration != nil {
+			if dep.Configuration != nil { //nolint:gocritic // switch is not suitable here
 				image = *dep.Configuration
 			} else if dep.Provider != nil {
 				image = *dep.Provider
+			} else if dep.Function != nil {
+				image = *dep.Function
+			}
+			if len(image) > 0 {
+				image = fmt.Sprintf(imageFmt, image, dep.Version)
+				m.deps[image] = true
 			}
-			image = fmt.Sprintf(imageFmt, image, dep.Version)
-			m.deps[image] = true
 		}
 	}
 

diff --git a/internal/controller/rbac/definition/roles.go b/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.

diff --git a/internal/controller/rbac/definition/roles_test.go b/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,
 						},
 					},

diff --git a/internal/xfn/function_runner.go b/internal/xfn/function_runner.go
@@ -164,7 +164,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{}
@@ -187,12 +187,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
 		}
@@ -202,6 +214,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.
@@ -221,9 +234,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
@@ -258,17 +269,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 {
@@ -280,28 +290,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
 }
diff --git a/test/e2e/manifests/apiextensions/composition/bind-existing-xr/setup/provider.yaml b/test/e2e/manifests/apiextensions/composition/bind-existing-xr/setup/provider.yaml
@@ -3,5 +3,5 @@ kind: Provider
 metadata:
   name: provider-nop
 spec:
-  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0
+  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1
   ignoreCrossplaneConstraints: true
diff --git a/test/e2e/manifests/apiextensions/composition/composition-selection/setup/provider.yaml b/test/e2e/manifests/apiextensions/composition/composition-selection/setup/provider.yaml
@@ -3,5 +3,5 @@ kind: Provider
 metadata:
   name: provider-nop
 spec:
-  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0
+  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1
   ignoreCrossplaneConstraints: true
diff --git a/test/e2e/manifests/apiextensions/composition/functions/setup/provider.yaml b/test/e2e/manifests/apiextensions/composition/functions/setup/provider.yaml
@@ -3,5 +3,5 @@ kind: Provider
 metadata:
   name: provider-nop
 spec:
-  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0
+  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1
   ignoreCrossplaneConstraints: true
diff --git a/test/e2e/manifests/apiextensions/composition/minimal/setup/provider.yaml b/test/e2e/manifests/apiextensions/composition/minimal/setup/provider.yaml
@@ -3,5 +3,5 @@ kind: Provider
 metadata:
   name: provider-nop
 spec:
-  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0
+  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1
   ignoreCrossplaneConstraints: true
diff --git a/test/e2e/manifests/apiextensions/composition/patch-and-transform/setup/provider.yaml b/test/e2e/manifests/apiextensions/composition/patch-and-transform/setup/provider.yaml
@@ -3,5 +3,5 @@ kind: Provider
 metadata:
   name: provider-nop
 spec:
-  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0
+  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1
   ignoreCrossplaneConstraints: true
diff --git a/test/e2e/manifests/apiextensions/composition/propagate-field-removals/setup/provider.yaml b/test/e2e/manifests/apiextensions/composition/propagate-field-removals/setup/provider.yaml
@@ -3,5 +3,5 @@ kind: Provider
 metadata:
   name: provider-nop
 spec:
-  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0
+  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1
   ignoreCrossplaneConstraints: true
diff --git a/test/e2e/manifests/apiextensions/composition/realtime-compositions/setup/provider.yaml b/test/e2e/manifests/apiextensions/composition/realtime-compositions/setup/provider.yaml
@@ -3,5 +3,5 @@ kind: Provider
 metadata:
   name: provider-nop
 spec:
-  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0
+  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1
   ignoreCrossplaneConstraints: true
diff --git a/test/e2e/manifests/apiextensions/composition/realtime-revision-selection/setup/provider.yaml b/test/e2e/manifests/apiextensions/composition/realtime-revision-selection/setup/provider.yaml
@@ -3,5 +3,5 @@ kind: Provider
 metadata:
   name: provider-nop
 spec:
-  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0
+  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1
   ignoreCrossplaneConstraints: true
diff --git a/test/e2e/manifests/apiextensions/composition/validation/setup/provider.yaml b/test/e2e/manifests/apiextensions/composition/validation/setup/provider.yaml
@@ -3,5 +3,5 @@ kind: Provider
 metadata:
   name: provider-nop
 spec:
-  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0
+  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1
   ignoreCrossplaneConstraints: true
diff --git a/test/e2e/manifests/apiextensions/environment/setup/provider.yaml b/test/e2e/manifests/apiextensions/environment/setup/provider.yaml
@@ -3,5 +3,5 @@ kind: Provider
 metadata:
   name: provider-nop
 spec:
-  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0
+  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1
   ignoreCrossplaneConstraints: true
diff --git a/test/e2e/manifests/lifecycle/upgrade/setup/provider.yaml b/test/e2e/manifests/lifecycle/upgrade/setup/provider.yaml
@@ -3,5 +3,5 @@ kind: Provider
 metadata:
   name: provider-nop
 spec:
-  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0
+  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1
   ignoreCrossplaneConstraints: true
diff --git a/test/e2e/manifests/pkg/deployment-runtime-config/setup/provider.yaml b/test/e2e/manifests/pkg/deployment-runtime-config/setup/provider.yaml
@@ -3,5 +3,5 @@ kind: Provider
 metadata:
   name: provider-nop
 spec:
-  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.0
+  package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1
   ignoreCrossplaneConstraints: true