diff --git a/.github/workflows/cd.yaml b/.github/workflows/cd.yaml new file mode 100644 index 00000000000..4c791e2342a --- /dev/null +++ b/.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 + diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 00000000000..b817369139e --- /dev/null +++ b/.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}}" diff --git a/.github/workflows/dependency-review.yaml b/.github/workflows/dependency-review.yaml new file mode 100644 index 00000000000..5eb64ee3c73 --- /dev/null +++ b/.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 diff --git a/cmd/controller/app/controller.go b/cmd/controller/app/controller.go index 4e38de81224..cba5594c559 100644 --- a/cmd/controller/app/controller.go +++ b/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, }, diff --git a/cmd/controller/app/options/options.go b/cmd/controller/app/options/options.go index 4798b247245..1d31864879a 100644 --- a/cmd/controller/app/options/options.go +++ b/cmd/controller/app/options/options.go @@ -164,6 +164,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. "+ diff --git a/internal/apis/config/controller/types.go b/internal/apis/config/controller/types.go index 9a8fcc570bb..6b36c2368e7 100644 --- a/internal/apis/config/controller/types.go +++ b/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 } diff --git a/internal/apis/config/controller/v1alpha1/defaults.go b/internal/apis/config/controller/v1alpha1/defaults.go index db4bf30a15f..600ffaf4a8e 100644 --- a/internal/apis/config/controller/v1alpha1/defaults.go +++ b/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) + } } diff --git a/internal/apis/config/controller/v1alpha1/testdata/defaults.json b/internal/apis/config/controller/v1alpha1/testdata/defaults.json index 9df951afe27..21bd6d411c1 100644 --- a/internal/apis/config/controller/v1alpha1/testdata/defaults.json +++ b/internal/apis/config/controller/v1alpha1/testdata/defaults.json @@ -65,6 +65,7 @@ }, "acmeDNS01Config": { "recursiveNameserversOnly": false, - "checkRetryPeriod": "10s" + "checkRetryPeriod": "10s", + "propagationTime": "1m0s" } } \ No newline at end of file diff --git a/internal/apis/config/controller/v1alpha1/zz_generated.conversion.go b/internal/apis/config/controller/v1alpha1/zz_generated.conversion.go index 446cdc3f927..da91bcbf34f 100644 --- a/internal/apis/config/controller/v1alpha1/zz_generated.conversion.go +++ b/internal/apis/config/controller/v1alpha1/zz_generated.conversion.go @@ -100,6 +100,9 @@ func autoConvert_v1alpha1_ACMEDNS01Config_To_controller_ACMEDNS01Config(in *v1al if err := sharedv1alpha1.Convert_Pointer_v1alpha1_Duration_To_time_Duration(&in.CheckRetryPeriod, &out.CheckRetryPeriod, s); err != nil { return err } + if err := sharedv1alpha1.Convert_Pointer_v1alpha1_Duration_To_time_Duration(&in.PropagationTime, &out.PropagationTime, s); err != nil { + return err + } return nil } @@ -116,6 +119,9 @@ func autoConvert_controller_ACMEDNS01Config_To_v1alpha1_ACMEDNS01Config(in *cont if err := sharedv1alpha1.Convert_time_Duration_To_Pointer_v1alpha1_Duration(&in.CheckRetryPeriod, &out.CheckRetryPeriod, s); err != nil { return err } + if err := sharedv1alpha1.Convert_time_Duration_To_Pointer_v1alpha1_Duration(&in.PropagationTime, &out.PropagationTime, s); err != nil { + return err + } return nil } diff --git a/pkg/apis/config/controller/v1alpha1/types.go b/pkg/apis/config/controller/v1alpha1/types.go index 4b370a444ff..78f8ace55bb 100644 --- a/pkg/apis/config/controller/v1alpha1/types.go +++ b/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"` } diff --git a/pkg/apis/config/controller/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/config/controller/v1alpha1/zz_generated.deepcopy.go index 4f33ec23375..b56f01c7855 100644 --- a/pkg/apis/config/controller/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/config/controller/v1alpha1/zz_generated.deepcopy.go @@ -44,6 +44,11 @@ func (in *ACMEDNS01Config) DeepCopyInto(out *ACMEDNS01Config) { *out = new(sharedv1alpha1.Duration) **out = **in } + if in.PropagationTime != nil { + in, out := &in.PropagationTime, &out.PropagationTime + *out = new(sharedv1alpha1.Duration) + **out = **in + } return } diff --git a/pkg/controller/acmechallenges/scheduler/scheduler.go b/pkg/controller/acmechallenges/scheduler/scheduler.go index 73499bfcc1c..3d6afbb48ad 100644 --- a/pkg/controller/acmechallenges/scheduler/scheduler.go +++ b/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 diff --git a/pkg/controller/acmechallenges/scheduler/scheduler_test.go b/pkg/controller/acmechallenges/scheduler/scheduler_test.go index ce7b6bda3eb..06e03281ddd 100644 --- a/pkg/controller/acmechallenges/scheduler/scheduler_test.go +++ b/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, diff --git a/pkg/controller/context.go b/pkg/controller/context.go index 5b412195f84..d197bbae273 100644 --- a/pkg/controller/context.go +++ b/pkg/controller/context.go @@ -210,6 +210,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. diff --git a/pkg/issuer/acme/dns/dns.go b/pkg/issuer/acme/dns/dns.go index 239a7302ee3..a59af8d49cc 100644 --- a/pkg/issuer/acme/dns/dns.go +++ b/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 diff --git a/test/unit/gen/challenge.go b/test/unit/gen/challenge.go index b6217e5c426..e984bec3528 100644 --- a/test/unit/gen/challenge.go +++ b/test/unit/gen/challenge.go @@ -17,6 +17,8 @@ limitations under the License. package gen import ( + "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" cmacme "github.com/cert-manager/cert-manager/pkg/apis/acme/v1" @@ -80,6 +82,18 @@ func SetChallengeDNSName(dnsName string) ChallengeModifier { } } +func SetChallengeSolver(s cmacme.ACMEChallengeSolver) ChallengeModifier { + return func(ch *cmacme.Challenge) { + ch.Spec.Solver = s + } +} + +func SetChallengeCreationTimestamp(t time.Time) ChallengeModifier { + return func(ch *cmacme.Challenge) { + ch.CreationTimestamp.Time = t + } +} + func SetChallengePresented(p bool) ChallengeModifier { return func(ch *cmacme.Challenge) { ch.Status.Presented = p