Fix: fix the goroutine leak in http request (#4303)

Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com>
(cherry picked from commit 559ef83abd)

Co-authored-by: FogDong <dongtianxin.tx@alibaba-inc.com>

.github/workflows/registry.yml

@@ -44,28 +44,24 @@ jobs:
           registry: docker.io
           username: ${{ secrets.DOCKER_USERNAME }}
           password: ${{ secrets.DOCKER_PASSWORD }}
-      - name: Login Alibaba Cloud ACR
+      - name: Login kubevela private registry
         uses: docker/login-action@v1
         with:
-          registry: kubevela-registry.cn-hangzhou.cr.aliyuncs.com
-          username: ${{ secrets.ACR_USERNAME }}@aliyun-inner.com
+          registry: ${{ secrets.ACR_DOMAIN }}
+          username: ${{ secrets.ACR_USERNAME }}
           password: ${{ secrets.ACR_PASSWORD }}
       - uses: docker/setup-qemu-action@v1
       - uses: docker/setup-buildx-action@v1
         with:
           driver-opts: image=moby/buildkit:master
-
-      - name: Build & Pushing vela-core for ACR
-        run: |
-          docker build --build-arg GOPROXY=https://proxy.golang.org --build-arg VERSION=${{ steps.get_version.outputs.VERSION }} --build-arg GITVERSION=git-${{ steps.vars.outputs.git_revision }}  -t kubevela-registry.cn-hangzhou.cr.aliyuncs.com/oamdev/vela-core:${{ steps.get_version.outputs.VERSION }} .
-          docker push kubevela-registry.cn-hangzhou.cr.aliyuncs.com/oamdev/vela-core:${{ steps.get_version.outputs.VERSION }}
+      
       - uses: docker/build-push-action@v2
-        name: Build & Pushing vela-core for Dockerhub and GHCR
+        name: Build & Pushing vela-core
         with:
           context: .
           file: Dockerfile
           labels: |-
-            org.opencontainers.image.source=https://github.com/${{ github.repository }}
+            org.opencontainers.image.source=https://github.com/${{ github.repository_owner }}
             org.opencontainers.image.revision=${{ github.sha }}
           platforms: linux/amd64,linux/arm64
           push: ${{ github.event_name != 'pull_request' }}
@@ -75,19 +71,16 @@ jobs:
             GOPROXY=https://proxy.golang.org
           tags: |-
             docker.io/oamdev/vela-core:${{ steps.get_version.outputs.VERSION }}
-            ghcr.io/${{ github.repository }}/vela-core:${{ steps.get_version.outputs.VERSION }}
+            ghcr.io/${{ github.repository_owner }}/vela-core:${{ steps.get_version.outputs.VERSION }}
+            ${{ secrets.ACR_DOMAIN }}/oamdev/vela-core:${{ steps.get_version.outputs.VERSION }}
 
-      - name: Build & Pushing vela-apiserver for ACR
-        run: |
-          docker build --build-arg GOPROXY=https://proxy.golang.org --build-arg VERSION=${{ steps.get_version.outputs.VERSION }} --build-arg GITVERSION=git-${{ steps.vars.outputs.git_revision }} -t kubevela-registry.cn-hangzhou.cr.aliyuncs.com/oamdev/vela-apiserver:${{ steps.get_version.outputs.VERSION }} -f Dockerfile.apiserver .
-          docker push kubevela-registry.cn-hangzhou.cr.aliyuncs.com/oamdev/vela-apiserver:${{ steps.get_version.outputs.VERSION }}
       - uses: docker/build-push-action@v2
-        name: Build & Pushing vela-apiserver for Dockerhub and GHCR
+        name: Build & Pushing vela-apiserver
         with:
           context: .
           file: Dockerfile.apiserver
           labels: |-
-            org.opencontainers.image.source=https://github.com/${{ github.repository }}
+            org.opencontainers.image.source=https://github.com/${{ github.repository_owner }}
             org.opencontainers.image.revision=${{ github.sha }}
           platforms: linux/amd64,linux/arm64
           push: ${{ github.event_name != 'pull_request' }}
@@ -97,19 +90,16 @@ jobs:
             GOPROXY=https://proxy.golang.org
           tags: |-
             docker.io/oamdev/vela-apiserver:${{ steps.get_version.outputs.VERSION }}
-            ghcr.io/${{ github.repository }}/vela-apiserver:${{ steps.get_version.outputs.VERSION }}
+            ghcr.io/${{ github.repository_owner }}/vela-apiserver:${{ steps.get_version.outputs.VERSION }}
+            ${{ secrets.ACR_DOMAIN }}/oamdev/vela-apiserver:${{ steps.get_version.outputs.VERSION }}
 
-      - name: Build & Pushing vela runtime rollout for ACR
-        run: |
-          docker build --build-arg GOPROXY=https://proxy.golang.org --build-arg VERSION=${{ steps.get_version.outputs.VERSION }} --build-arg GITVERSION=git-${{ steps.vars.outputs.git_revision }}  -t kubevela-registry.cn-hangzhou.cr.aliyuncs.com/oamdev/vela-rollout:${{ steps.get_version.outputs.VERSION }} .
-          docker push kubevela-registry.cn-hangzhou.cr.aliyuncs.com/oamdev/vela-rollout:${{ steps.get_version.outputs.VERSION }}
       - uses: docker/build-push-action@v2
-        name: Build & Pushing runtime rollout for Dockerhub and GHCR
+        name: Build & Pushing runtime rollout
         with:
           context: .
           file: runtime/rollout/Dockerfile
           labels: |-
-            org.opencontainers.image.source=https://github.com/${{ github.repository }}
+            org.opencontainers.image.source=https://github.com/${{ github.repository_owner }}
             org.opencontainers.image.revision=${{ github.sha }}
           platforms: linux/amd64,linux/arm64
           push: ${{ github.event_name != 'pull_request' }}
@@ -119,7 +109,8 @@ jobs:
             GOPROXY=https://proxy.golang.org
           tags: |-
             docker.io/oamdev/vela-rollout:${{ steps.get_version.outputs.VERSION }}
-            ghcr.io/${{ github.repository }}/vela-rollout:${{ steps.get_version.outputs.VERSION }}
+            ghcr.io/${{ github.repository_owner }}/vela-rollout:${{ steps.get_version.outputs.VERSION }}
+            ${{ secrets.ACR_DOMAIN }}/oamdev/vela-rollout:${{ steps.get_version.outputs.VERSION }}
 
   publish-charts:
     env:

charts/vela-core/templates/defwithtemplate/command.yaml

@@ -106,7 +106,7 @@ spec:
         		}]
         	}
         }
-        parameter: #PatchParams | close({
+        parameter: *#PatchParams | close({
         	// +usage=Specify the commands for multiple containers
         	containers: [...#PatchParams]
         })

charts/vela-core/templates/defwithtemplate/container-image.yaml

@@ -69,7 +69,7 @@ spec:
         		}]
         	}
         }
-        parameter: #PatchParams | close({
+        parameter: *#PatchParams | close({
         	// +usage=Specify the container image for multiple containers
         	containers: [...#PatchParams]
         })

charts/vela-core/templates/defwithtemplate/env.yaml

@@ -62,7 +62,8 @@ spec:
         					}
         				}
         			}] + [ for k, v in _params.env if _delKeys[k] == _|_ && (_params.replace || _baseEnvMap[k] == _|_) {
-        				v
+        				name:  k
+        				value: v
         			}]
         		}
         	}
@@ -96,7 +97,7 @@ spec:
         		}]
         	}
         }
-        parameter: #PatchParams | close({
+        parameter: *#PatchParams | close({
         	// +usage=Specify the environment variables for multiple containers
         	containers: [...#PatchParams]
         })

charts/vela-minimal/templates/defwithtemplate/command.yaml

@@ -106,7 +106,7 @@ spec:
         		}]
         	}
         }
-        parameter: #PatchParams | close({
+        parameter: *#PatchParams | close({
         	// +usage=Specify the commands for multiple containers
         	containers: [...#PatchParams]
         })

charts/vela-minimal/templates/defwithtemplate/container-image.yaml

@@ -69,7 +69,7 @@ spec:
         		}]
         	}
         }
-        parameter: #PatchParams | close({
+        parameter: *#PatchParams | close({
         	// +usage=Specify the container image for multiple containers
         	containers: [...#PatchParams]
         })

charts/vela-minimal/templates/defwithtemplate/env.yaml

@@ -62,7 +62,8 @@ spec:
         					}
         				}
         			}] + [ for k, v in _params.env if _delKeys[k] == _|_ && (_params.replace || _baseEnvMap[k] == _|_) {
-        				v
+        				name:  k
+        				value: v
         			}]
         		}
         	}
@@ -96,7 +97,7 @@ spec:
         		}]
         	}
         }
-        parameter: #PatchParams | close({
+        parameter: *#PatchParams | close({
         	// +usage=Specify the environment variables for multiple containers
         	containers: [...#PatchParams]
         })

pkg/builtin/http/http.go

@@ -51,7 +51,7 @@ func (c *HTTPCmd) Run(meta *registry.Meta) (res interface{}, err error) {
 	var (
 		r      io.Reader
 		client = &http.Client{
-			Transport: &http.Transport{},
+			Transport: http.DefaultTransport,
 			Timeout:   time.Second * 3,
 		}
 	)

pkg/controller/core.oam.dev/v1alpha2/application/application_controller_test.go

@@ -3897,7 +3897,7 @@ spec:
         		}]
         	}
         }
-        parameter: #PatchParams | close({
+        parameter: *#PatchParams | close({
         	// +usage=Specify the environment variables for multiple containers
         	containers: [...#PatchParams]
         })

pkg/oam/labels.go

@@ -117,7 +117,7 @@ const (
 	AnnotationAppGeneration = "app.oam.dev/generation"
 
 	// AnnotationLastAppliedConfig records the previous configuration of a
-	// resource for use in a three way diff during a patching apply
+	// resource for use in a three-way diff during a patching apply
 	AnnotationLastAppliedConfig = "app.oam.dev/last-applied-configuration"
 
 	// AnnotationLastAppliedTime indicates the last applied time

pkg/utils/apply/apply.go

@@ -20,19 +20,19 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
-	"github.com/oam-dev/kubevela/pkg/controller/utils"
-	"github.com/oam-dev/kubevela/pkg/oam/util"
-
-	"github.com/oam-dev/kubevela/pkg/oam"
-
 	"github.com/pkg/errors"
+	corev1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
+	"github.com/oam-dev/kubevela/pkg/controller/utils"
+	"github.com/oam-dev/kubevela/pkg/oam"
+	"github.com/oam-dev/kubevela/pkg/oam/util"
 )
 
 const (
@@ -107,13 +107,42 @@ func loggingApply(msg string, desired client.Object) {
 	klog.InfoS(msg, "name", d.GetName(), "resource", desired.GetObjectKind().GroupVersionKind().String())
 }
 
+// filterRecordForSpecial will filter special object that can reduce the record for "app.oam.dev/last-applied-configuration" annotation.
+func filterRecordForSpecial(desired client.Object) bool {
+	if desired == nil {
+		return false
+	}
+	gvk := desired.GetObjectKind().GroupVersionKind()
+	gp, kd := gvk.Group, gvk.Kind
+	if gp == "" {
+		// group is empty means it's Kubernetes core API, we won't record annotation for Secret and Configmap
+		if kd == "Secret" || kd == "ConfigMap" {
+			return false
+		}
+		if _, ok := desired.(*corev1.ConfigMap); ok {
+			return false
+		}
+		if _, ok := desired.(*corev1.Secret); ok {
+			return false
+		}
+	}
+	ann := desired.GetAnnotations()
+	if ann != nil {
+		lac := ann[oam.AnnotationLastAppliedConfig]
+		if lac == "-" || lac == "skip" {
+			return false
+		}
+	}
+	return true
+}
+
 // Apply applies new state to an object or create it if not exist
 func (a *APIApplicator) Apply(ctx context.Context, desired client.Object, ao ...ApplyOption) error {
 	_, err := generateRenderHash(desired)
 	if err != nil {
 		return err
 	}
-	applyAct := &applyAction{updateAnnotation: true}
+	applyAct := &applyAction{updateAnnotation: filterRecordForSpecial(desired)}
 	existing, err := a.createOrGetExisting(ctx, applyAct, a.c, desired, ao...)
 	if err != nil {
 		return err

pkg/utils/apply/apply_test.go

@@ -23,8 +23,10 @@ import (
 	"github.com/crossplane/crossplane-runtime/pkg/test"
 	"github.com/google/go-cmp/cmp"
 	"github.com/pkg/errors"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -423,3 +425,19 @@ func TestMustBeControlledByApp(t *testing.T) {
 		})
 	}
 }
+
+func TestFilterSpecialAnn(t *testing.T) {
+	var cm = &corev1.ConfigMap{}
+	var sc = &corev1.Secret{}
+	var dp = &appsv1.Deployment{}
+	assert.Equal(t, false, filterRecordForSpecial(cm))
+	assert.Equal(t, false, filterRecordForSpecial(sc))
+	assert.Equal(t, true, filterRecordForSpecial(dp))
+
+	dp.Annotations = map[string]string{oam.AnnotationLastAppliedConfig: "-"}
+	assert.Equal(t, false, filterRecordForSpecial(dp))
+	dp.Annotations = map[string]string{oam.AnnotationLastAppliedConfig: "skip"}
+	assert.Equal(t, false, filterRecordForSpecial(dp))
+	dp.Annotations = map[string]string{oam.AnnotationLastAppliedConfig: "xxx"}
+	assert.Equal(t, true, filterRecordForSpecial(dp))
+}

pkg/utils/common/common.go

@@ -37,6 +37,7 @@ import (
 	"github.com/hashicorp/hcl/v2/hclparse"
 	clustergatewayapi "github.com/oam-dev/cluster-gateway/pkg/apis/cluster/v1alpha1"
 	"github.com/oam-dev/terraform-config-inspect/tfconfig"
+	terraformapiv1 "github.com/oam-dev/terraform-controller/api/v1beta1"
 	terraformapi "github.com/oam-dev/terraform-controller/api/v1beta2"
 	kruise "github.com/openkruise/kruise-api/apps/v1alpha1"
 	errors2 "github.com/pkg/errors"
@@ -49,6 +50,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
+	metricsV1beta1api "k8s.io/metrics/pkg/apis/metrics/v1beta1"
 	ocmclusterv1 "open-cluster-management.io/api/cluster/v1"
 	ocmclusterv1alpha1 "open-cluster-management.io/api/cluster/v1alpha1"
 	ocmworkv1 "open-cluster-management.io/api/work/v1"
@@ -56,8 +58,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/yaml"
 
-	metricsV1beta1api "k8s.io/metrics/pkg/apis/metrics/v1beta1"
-
 	oamcore "github.com/oam-dev/kubevela/apis/core.oam.dev"
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/common"
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
@@ -94,6 +94,7 @@ func init() {
 	_ = certmanager.AddToScheme(Scheme)
 	_ = kruise.AddToScheme(Scheme)
 	_ = terraformapi.AddToScheme(Scheme)
+	_ = terraformapiv1.AddToScheme(Scheme)
 	_ = ocmclusterv1alpha1.Install(Scheme)
 	_ = ocmclusterv1.Install(Scheme)
 	_ = ocmworkv1.Install(Scheme)

references/cli/traits_test.go

@@ -17,11 +17,22 @@ limitations under the License.
 package cli
 
 import (
+	"bytes"
+	"context"
+	"os"
+	"strings"
 	"testing"
 
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
 	"github.com/stretchr/testify/assert"
+	"sigs.k8s.io/yaml"
 
+	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
 	"github.com/oam-dev/kubevela/apis/types"
+	util2 "github.com/oam-dev/kubevela/pkg/oam/util"
+	common2 "github.com/oam-dev/kubevela/pkg/utils/common"
+	"github.com/oam-dev/kubevela/pkg/utils/util"
 	"github.com/oam-dev/kubevela/references/common"
 )
 
@@ -43,3 +54,170 @@ func TestTraitsAppliedToAllWorkloads(t *testing.T) {
 	}
 	assert.Equal(t, []string{"*"}, common.ConvertApplyTo(trait.AppliesTo, workloads))
 }
+
+var _ = Describe("Test trait cli", func() {
+
+	When("there are container-image and configmap traits", func() {
+		BeforeEach(func() {
+			// Install trait locally
+			containerImage := v1beta1.TraitDefinition{}
+			Expect(yaml.Unmarshal([]byte(containerImageYaml), &containerImage)).Should(BeNil())
+			Expect(k8sClient.Create(context.Background(), &containerImage)).Should(SatisfyAny(BeNil(), util2.AlreadyExistMatcher{}))
+
+			configMap := v1beta1.TraitDefinition{}
+			Expect(yaml.Unmarshal([]byte(configmapYaml), &configMap)).Should(BeNil())
+			Expect(k8sClient.Create(context.Background(), &configMap)).Should(SatisfyAny(BeNil(), util2.AlreadyExistMatcher{}))
+		})
+
+		It("should not have any err", func() {
+			arg := common2.Args{}
+			arg.SetClient(k8sClient)
+			buffer := bytes.NewBuffer(nil)
+			ioStreams := util.IOStreams{In: os.Stdin, Out: buffer, ErrOut: buffer}
+			cmd := NewTraitCommand(arg, ioStreams)
+			Expect(cmd.Execute()).Should(BeNil())
+			buf, ok := ioStreams.Out.(*bytes.Buffer)
+			Expect(ok).Should(BeTrue())
+			Expect(strings.Contains(buf.String(), "error")).Should(BeFalse())
+		})
+	})
+})
+
+const (
+	containerImageYaml = `apiVersion: core.oam.dev/v1beta1
+kind: TraitDefinition
+metadata:
+  annotations:
+    definition.oam.dev/description: Set the image of the container.
+  name: container-image
+  namespace: vela-system
+spec:
+  appliesToWorkloads:
+    - '*'
+  podDisruptive: true
+  schematic:
+    cue:
+      template: |
+        #PatchParams: {
+        	// +usage=Specify the name of the target container, if not set, use the component name
+        	containerName: *"" | string
+        	// +usage=Specify the image of the container
+        	image: string
+        	// +usage=Specify the image pull policy of the container
+        	imagePullPolicy: *"" | "IfNotPresent" | "Always" | "Never"
+        }
+        PatchContainer: {
+        	_params:         #PatchParams
+        	name:            _params.containerName
+        	_baseContainers: context.output.spec.template.spec.containers
+        	_matchContainers_: [ for _container_ in _baseContainers if _container_.name == name {_container_}]
+        	_baseContainer: *_|_ | {...}
+        	if len(_matchContainers_) == 0 {
+        		err: "container \(name) not found"
+        	}
+        	if len(_matchContainers_) > 0 {
+        		// +patchStrategy=retainKeys
+        		image: _params.image
+
+        		if _params.imagePullPolicy != "" {
+        			// +patchStrategy=retainKeys
+        			imagePullPolicy: _params.imagePullPolicy
+        		}
+        	}
+        }
+        patch: spec: template: spec: {
+        	if parameter.containers == _|_ {
+        		// +patchKey=name
+        		containers: [{
+        			PatchContainer & {_params: {
+        				if parameter.containerName == "" {
+        					containerName: context.name
+        				}
+        				if parameter.containerName != "" {
+        					containerName: parameter.containerName
+        				}
+        				image:           parameter.image
+        				imagePullPolicy: parameter.imagePullPolicy
+        			}}
+        		}]
+        	}
+        	if parameter.containers != _|_ {
+        		// +patchKey=name
+        		containers: [ for c in parameter.containers {
+        			if c.containerName == "" {
+        				err: "containerName must be set for containers"
+        			}
+        			if c.containerName != "" {
+        				PatchContainer & {_params: c}
+        			}
+        		}]
+        	}
+        }
+        parameter: *#PatchParams | close({
+        	// +usage=Specify the container image for multiple containers
+        	containers: [...#PatchParams]
+        })
+        errs: [ for c in patch.spec.template.spec.containers if c.err != _|_ {c.err}]
+`
+	configmapYaml = `apiVersion: core.oam.dev/v1beta1
+kind: TraitDefinition
+metadata:
+  annotations:
+    definition.oam.dev/description: Create/Attach configmaps on K8s pod for your workload which follows the pod spec in path 'spec.template'. This definition is DEPRECATED, please specify configmap in 'storage' instead.
+  labels:
+    custom.definition.oam.dev/deprecated: "true"
+  name: configmap
+  namespace: vela-system
+spec:
+  appliesToWorkloads:
+    - '*'
+  podDisruptive: true
+  schematic:
+    cue:
+      template: |
+        patch: spec: template: spec: {
+        	containers: [{
+        		// +patchKey=name
+        		volumeMounts: [
+        			for v in parameter.volumes {
+        				{
+        					name:      "volume-\(v.name)"
+        					mountPath: v.mountPath
+        					readOnly:  v.readOnly
+        				}
+        			},
+        		]
+        	}, ...]
+        	// +patchKey=name
+        	volumes: [
+        		for v in parameter.volumes {
+        			{
+        				name: "volume-\(v.name)"
+        				configMap: name: v.name
+        			}
+        		},
+        	]
+        }
+        outputs: {
+        	for v in parameter.volumes {
+        		if v.data != _|_ {
+        			"\(v.name)": {
+        				apiVersion: "v1"
+        				kind:       "ConfigMap"
+        				metadata: name: v.name
+        				data: v.data
+        			}
+        		}
+        	}
+        }
+        parameter: {
+        	// +usage=Specify mounted configmap names and their mount paths in the container
+        	volumes: [...{
+        		name:      string
+        		mountPath: string
+        		readOnly:  *false | bool
+        		data?: [string]: string
+        	}]
+        }
+`
+)

vela-templates/definitions/internal/trait/command.cue

@@ -100,7 +100,7 @@ template: {
 		}
 	}
 
-	parameter: #PatchParams | close({
+	parameter: *#PatchParams | close({
 		// +usage=Specify the commands for multiple containers
 		containers: [...#PatchParams]
 	})

vela-templates/definitions/internal/trait/container-image.cue

@@ -65,7 +65,7 @@ template: {
 		}
 	}
 
-	parameter: #PatchParams | close({
+	parameter: *#PatchParams | close({
 		// +usage=Specify the container image for multiple containers
 		containers: [...#PatchParams]
 	})

vela-templates/definitions/internal/trait/env.cue

@@ -55,7 +55,8 @@ template: {
 						}
 					}
 				}] + [ for k, v in _params.env if _delKeys[k] == _|_ && (_params.replace || _baseEnvMap[k] == _|_) {
-					v
+					name:  k
+					value: v
 				}]
 			}
 		}
@@ -90,7 +91,7 @@ template: {
 		}
 	}
 
-	parameter: #PatchParams | close({
+	parameter: *#PatchParams | close({
 		// +usage=Specify the environment variables for multiple containers
 		containers: [...#PatchParams]
 	})