Merge branch 'teleport' into mjsmith/v1.16.1-sync

.github/workflows/cd.yaml

@@ -0,0 +1,92 @@
+name: CI/CD
+on:
+  push:
+
+permissions:
+  contents: read
+
+jobs:
+  test:
+    name: Test application
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repo
+        uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4
+      - name: Setup Go
+        uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5
+        with:
+          go-version-file: go.mod
+          cache: true
+      - name: Run tests
+        run: make test VERSION=0.0.0
+  build-and-push:
+    name: Build and push Docker image
+    runs-on: ubuntu-latest
+    needs: [test]
+    env:
+      AWS_REGION_PUBLIC: us-east-1
+      AWS_ROLE: arn:aws:iam::146628656107:role/cert-manager-controller-github-action-ecr-role
+      # push images only for tags like vX.X.X-teleport
+      PUSH_IMAGE: ${{ (startsWith(github.ref, 'refs/tags/v') && contains(github.ref, 'teleport')) }}
+      ECR_REPO: public.ecr.aws/gravitational/cert-manager-controller
+    permissions:
+      contents: read
+      id-token: write # This is required for requesting the JWT, see https://github.com/aws-actions/configure-aws-credentials#OIDC
+    steps:
+      - name: Checkout repo
+        uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4
+        with:
+          fetch-depth: 0
+
+      - name: Setup Go
+        uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5
+        with:
+          go-version-file: go.mod
+          cache: true
+
+      - name: Configure AWS credentials for ECR Public
+        uses: aws-actions/configure-aws-credentials@e3dd6a429d7300a6a4c196c26e071d42e0343502 # v4
+        with:
+          aws-region: ${{ env.AWS_REGION_PUBLIC }}
+          role-to-assume: ${{ env.AWS_ROLE }}  
+        if: ${{ env.PUSH_IMAGE == 'true' }}
+
+      - name: Login to Amazon ECR Public
+        id: login-ecr-public
+        uses: aws-actions/amazon-ecr-login@062b18b96a7aff071d4dc91bc00c4c1a7945b076 # v2
+        with:
+          registry-type: public
+        if: ${{ env.PUSH_IMAGE == 'true' }}
+
+      - name: Prepare docker labels and tags
+        id: meta
+        uses: docker/metadata-action@8e5442c4ef9f78752691e2d8f8d19755c6f78e81 # v5
+        with:
+          images: |
+            ${{ env.ECR_REPO }}
+          flavor: |
+            latest=false
+          tags: |
+            type=sha,prefix={{branch}}-,suffix=-{{date 'YYYYMMDDTHHmmss'}},format=short,enable=${{ startsWith(github.ref, 'refs/heads/') }}
+            type=semver,pattern={{version}},enable=${{ github.event_name == 'push' && startsWith(github.ref, 'refs/tags/v') }}
+  
+      - name: Build [and push] multiarch docker image
+        # because cert-manager build is not using buildx for simpler multiarch builds,
+        # but produces image per arch, we need to build them one by one and combine into multiarch-manifest
+        shell: bash
+        run: |
+          # https://docs.docker.com/reference/cli/docker/manifest/#create-and-push-a-manifest-list
+          for arch in amd64 arm64 s390x ppc64le arm; do
+            tag=${{ steps.meta.outputs.tags }}-$arch-linux
+            make _bin/containers/cert-manager-controller-linux-$arch.tar TAG=$tag
+
+            if [ "$PUSH_IMAGE" = "true" ]; then
+              docker push $tag
+              docker manifest create -a ${{ steps.meta.outputs.tags }} $tag
+            fi
+          done
+         
+          if [ "$PUSH_IMAGE" = "true" ]; then
+            docker manifest push ${{ steps.meta.outputs.tags }}
+          fi
+

.github/workflows/codeql.yml

@@ -0,0 +1,47 @@
+name: "CodeQL"
+
+on:
+  push:
+    branches:
+      - teleport
+  pull_request:
+    branches:
+      - teleport
+
+jobs:
+  analyze:
+    name: Analyze
+    runs-on: ubuntu-latest
+    permissions:
+      actions: read
+      contents: read
+      security-events: write
+
+    strategy:
+      fail-fast: false
+      matrix:
+        language: [ 'go' ]
+
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v4
+
+    - name: Initialize CodeQL
+      uses: github/codeql-action/init@v2
+      with:
+        languages: ${{ matrix.language }}
+
+    - name: Set up Go
+      uses: actions/setup-go@v5
+      with:
+        cache: false
+        go-version-file: go.mod
+      if: ${{ matrix.language == 'go' }}
+
+    - name: Autobuild
+      uses: github/codeql-action/autobuild@v3
+
+    - name: Perform CodeQL Analysis
+      uses: github/codeql-action/analyze@v3
+      with:
+        category: "/language:${{matrix.language}}"

.github/workflows/dependency-review.yaml

@@ -0,0 +1,10 @@
+name: Dependency Review
+
+on:
+  pull_request:
+
+jobs:
+  dependency-review:
+    uses: gravitational/shared-workflows/.github/workflows/dependency-review.yaml@main
+    permissions:
+      contents: read

cmd/controller/app/controller.go

@@ -330,6 +330,7 @@ func buildControllerContextFactory(ctx context.Context, opts *config.ControllerC
 			DNS01Nameservers:        nameservers,
 			DNS01CheckRetryPeriod:   opts.ACMEDNS01Config.CheckRetryPeriod,
 			DNS01CheckAuthoritative: !opts.ACMEDNS01Config.RecursiveNameserversOnly,
+			DNS01PropagationTime:    opts.ACMEDNS01Config.PropagationTime,
 
 			AccountRegistry: acmeAccountRegistry,
 		},

cmd/controller/app/options/options.go

@@ -165,6 +165,9 @@ func AddConfigFlags(fs *pflag.FlagSet, c *config.ControllerConfiguration) {
 	fs.DurationVar(&c.ACMEDNS01Config.CheckRetryPeriod, "dns01-check-retry-period", c.ACMEDNS01Config.CheckRetryPeriod, ""+
 		"The duration the controller should wait between a propagation check. Despite the name, this flag is used to configure the wait period for both DNS01 and HTTP01 challenge propagation checks. For DNS01 challenges the propagation check verifies that a TXT record with the challenge token has been created. For HTTP01 challenges the propagation check verifies that the challenge token is served at the challenge URL."+
 		"This should be a valid duration string, for example 180s or 1h")
+	fs.DurationVar(&c.ACMEDNS01Config.PropagationTime, "dns01-propagation-time", c.ACMEDNS01Config.PropagationTime, ""+
+		"The duration the controller should wait after determining that an ACME dns entry exists."+
+		"This should be a valid duration string, for example 180s or 1h")
 
 	fs.BoolVar(&c.EnableCertificateOwnerRef, "enable-certificate-owner-ref", c.EnableCertificateOwnerRef, ""+
 		"Whether to set the certificate resource as an owner of secret where the tls certificate is stored. "+

internal/apis/config/controller/types.go

@@ -218,4 +218,8 @@ type ACMEDNS01Config struct {
 	// token is served at the challenge URL. This should be a valid duration
 	// string, for example 180s or 1h
 	CheckRetryPeriod time.Duration
+
+	// The duration the controller should wait after determining that an ACME dns entry exists.
+	// This should be a valid duration string, for example 180s or 1h
+	PropagationTime time.Duration
 }

internal/apis/config/controller/v1alpha1/defaults.go

@@ -77,6 +77,7 @@ var (
 	defaultDNS01RecursiveNameserversOnly = false
 	defaultDNS01RecursiveNameservers     = []string{}
 	defaultDNS01CheckRetryPeriod         = 10 * time.Second
+	defaultDNS01PropagationTime          = 60 * time.Second
 
 	defaultNumberOfConcurrentWorkers int32 = 5
 	defaultMaxConcurrentChallenges   int32 = 60
@@ -310,4 +311,8 @@ func SetDefaults_ACMEDNS01Config(obj *v1alpha1.ACMEDNS01Config) {
 	if obj.CheckRetryPeriod.IsZero() {
 		obj.CheckRetryPeriod = sharedv1alpha1.DurationFromTime(defaultDNS01CheckRetryPeriod)
 	}
+
+	if obj.PropagationTime.IsZero() {
+		obj.PropagationTime = sharedv1alpha1.DurationFromTime(defaultDNS01PropagationTime)
+	}
 }

internal/apis/config/controller/v1alpha1/testdata/defaults.json

@@ -65,6 +65,7 @@
 	},
 	"acmeDNS01Config": {
 		"recursiveNameserversOnly": false,
-		"checkRetryPeriod": "10s"
+		"checkRetryPeriod": "10s",
+		"propagationTime": "1m0s"
 	}
 }

pkg/apis/config/controller/v1alpha1/types.go

@@ -220,4 +220,8 @@ type ACMEDNS01Config struct {
 	// token is served at the challenge URL. This should be a valid duration
 	// string, for example 180s or 1h
 	CheckRetryPeriod *sharedv1alpha1.Duration `json:"checkRetryPeriod,omitempty"`
+
+	// The duration the controller should wait after determining that an ACME dns entry exists.
+	// This should be a valid duration string, for example 180s or 1h
+	PropagationTime *sharedv1alpha1.Duration `json:"propagationTime,omitempty"`
 }

pkg/controller/acmechallenges/scheduler/scheduler.go

@@ -192,6 +192,25 @@ func compareChallenges(l, r *cmacme.Challenge) int {
 		return 1
 	}
 
+	// Explicitly allow parallel DNS-01 reqs for the same DNSNames using Route53,
+	// as the route53 package is capable of handling these.
+	// We could assume different solvers are safe to schedule in parallel,
+	// but this might not always be true (e.g., with custom webhooks)
+	if l.Spec.Type == cmacme.ACMEChallengeTypeDNS01 &&
+		l.Spec.Solver.DNS01 != nil &&
+		l.Spec.Solver.DNS01.Route53 != nil &&
+		r.Spec.Solver.DNS01 != nil &&
+		r.Spec.Solver.DNS01.Route53 != nil {
+
+		// Use key to guarantee that different requests are never equal
+		if l.Spec.Key < r.Spec.Key {
+			return -1
+		}
+		if l.Spec.Key > r.Spec.Key {
+			return 1
+		}
+	}
+
 	// TODO: check the http01.ingressClass attribute and allow two challenges
 	// with different ingress classes specified to be scheduled at once
 

pkg/controller/acmechallenges/scheduler/scheduler_test.go

@@ -264,6 +264,64 @@ func TestScheduleN(t *testing.T) {
 					gen.SetChallengeWildcard(true)),
 			},
 		},
+		{
+			name: "schedule parallel DNS-01 challenges when using route53",
+			n:    5,
+			challenges: []*cmacme.Challenge{
+				gen.Challenge("test1",
+					gen.SetChallengeDNSName("example.com"),
+					gen.SetChallengeType(cmacme.ACMEChallengeTypeDNS01),
+					gen.SetChallengeSolver(cmacme.ACMEChallengeSolver{
+						DNS01: &cmacme.ACMEChallengeSolverDNS01{
+							Route53: &cmacme.ACMEIssuerDNS01ProviderRoute53{},
+						},
+					}),
+					gen.SetChallengeKey("2"),
+				),
+				gen.Challenge("test2",
+					gen.SetChallengeDNSName("example.com"),
+					gen.SetChallengeType(cmacme.ACMEChallengeTypeDNS01),
+					gen.SetChallengeSolver(cmacme.ACMEChallengeSolver{
+						DNS01: &cmacme.ACMEChallengeSolverDNS01{
+							Route53: &cmacme.ACMEIssuerDNS01ProviderRoute53{},
+						},
+					}),
+					gen.SetChallengeKey("1"),
+				),
+				gen.Challenge("test3",
+					gen.SetChallengeDNSName("example.com"),
+					gen.SetChallengeType(cmacme.ACMEChallengeTypeDNS01),
+					gen.SetChallengeSolver(cmacme.ACMEChallengeSolver{
+						DNS01: &cmacme.ACMEChallengeSolverDNS01{
+							Cloudflare: &cmacme.ACMEIssuerDNS01ProviderCloudflare{},
+						},
+					}),
+					gen.SetChallengeCreationTimestamp(time.Unix(1, 1)),
+				),
+			},
+			expected: []*cmacme.Challenge{
+				gen.Challenge("test2",
+					gen.SetChallengeDNSName("example.com"),
+					gen.SetChallengeType(cmacme.ACMEChallengeTypeDNS01),
+					gen.SetChallengeSolver(cmacme.ACMEChallengeSolver{
+						DNS01: &cmacme.ACMEChallengeSolverDNS01{
+							Route53: &cmacme.ACMEIssuerDNS01ProviderRoute53{},
+						},
+					}),
+					gen.SetChallengeKey("1"),
+				),
+				gen.Challenge("test1",
+					gen.SetChallengeDNSName("example.com"),
+					gen.SetChallengeType(cmacme.ACMEChallengeTypeDNS01),
+					gen.SetChallengeSolver(cmacme.ACMEChallengeSolver{
+						DNS01: &cmacme.ACMEChallengeSolverDNS01{
+							Route53: &cmacme.ACMEIssuerDNS01ProviderRoute53{},
+						},
+					}),
+					gen.SetChallengeKey("2"),
+				),
+			},
+		},
 		{
 			name: "don't schedule when total number of scheduled challenges exceeds global maximum",
 			n:    5,

pkg/controller/context.go

@@ -211,6 +211,9 @@ type ACMEOptions struct {
 
 	// DNS01CheckRetryPeriod is the time the controller should wait between checking if a ACME dns entry exists.
 	DNS01CheckRetryPeriod time.Duration
+
+	// The duration the controller should wait after determining that an ACME dns entry exists.
+	DNS01PropagationTime time.Duration
 }
 
 // IngressShimOptions contain default Issuer GVK config for the certificate-shim controllers.

pkg/issuer/acme/dns/dns.go

@@ -126,9 +126,10 @@ func (s *Solver) Check(ctx context.Context, issuer v1.GenericIssuer, ch *cmacme.
 		return fmt.Errorf("DNS record for %q not yet propagated", ch.Spec.DNSName)
 	}
 
-	ttl := 60
-	log.V(logf.DebugLevel).Info("waiting DNS record TTL to allow the DNS01 record to propagate for domain", "ttl", ttl, "fqdn", fqdn)
-	time.Sleep(time.Second * time.Duration(ttl))
+	if s.DNS01PropagationTime > 0 {
+		log.V(logf.DebugLevel).Info("waiting DNS01 record to propagate for domain", "duration", s.DNS01PropagationTime, "fqdn", fqdn)
+		time.Sleep(s.DNS01PropagationTime)
+	}
 	log.V(logf.DebugLevel).Info("ACME DNS01 validation record propagated", "fqdn", fqdn)
 
 	return nil

pkg/issuer/acme/dns/route53/route53.go

@@ -146,6 +146,13 @@ func (d *sessionProvider) GetSession(ctx context.Context) (aws.Config, error) {
 		return aws.Config{}, fmt.Errorf("unable to create aws config: %s", err)
 	}
 
+	// For backwards compatibility with cert-manager <= 1.14, where we used the aws-sdk-go v1
+	// library, we configure the SDK here to use the global sts endpoint. This was the default
+	// behaviour of the SDK v1 library, but has to be explicitly set in the v2 library. For the
+	// route53 calls, we use the region provided by the user (see below).
+	stsCfg := cfg.Copy()
+	stsCfg.Region = "aws-global"
+
 	if d.Role != "" && d.WebIdentityToken == "" {
 		log.V(logf.DebugLevel).WithValues("role", d.Role).Info("assuming role")
 		stsSvc := d.StsProvider(cfg)
@@ -167,7 +174,7 @@ func (d *sessionProvider) GetSession(ctx context.Context) (aws.Config, error) {
 	if d.Role != "" && d.WebIdentityToken != "" {
 		log.V(logf.DebugLevel).WithValues("role", d.Role).Info("assuming role with web identity")
 
-		stsSvc := d.StsProvider(cfg)
+		stsSvc := d.StsProvider(stsCfg)
 		result, err := stsSvc.AssumeRoleWithWebIdentity(ctx, &sts.AssumeRoleWithWebIdentityInput{
 			RoleArn:          aws.String(d.Role),
 			RoleSessionName:  aws.String("cert-manager"),