Enable Let's Encrypt error handling for all pages domains (!28784) · Merge requests · GitLab.org / GitLab

Vladimir Shushlin requested to merge 30146-let-s-encrypt-integration-doesn-t-scale-and-does-not-give-any-feedback-to-user-on-errors-4 into master Apr 03, 2020

What does this MR do?

This is WIP until it's been tested on gitlab.com, but I want to it be ready for merge

The final piece of #30146 (closed): removing feature flag.

Also moving filter from cron worker to scope and creating an index.

From database-lab https://gitlab.slack.com/archives/CLJMDRD8C/p1586168415393700:

exec CREATE INDEX index_pages_domains_need_auto_ssl_renewal_user_provided ON public.pages_domains USING btree (id) WHERE ((auto_ssl_enabled = true) AND (auto_ssl_failed = false) AND (certificate_source = 0));

CREATE INDEX index_pages_domains_need_auto_ssl_renewal_valid_not_after ON public.pages_domains USING btree (certificate_valid_not_after) WHERE ((auto_ssl_enabled = true) AND (auto_ssl_failed = false));

explain SELECT "pages_domains".* FROM ((SELECT "pages_domains".* FROM "pages_domains" WHERE "pages_domains"."auto_ssl_enabled" = TRUE AND "pages_domains"."auto_ssl_failed" = FALSE AND "pages_domains"."certificate_source" = 0)
UNION
(SELECT "pages_domains".* FROM "pages_domains" WHERE "pages_domains"."auto_ssl_enabled" = TRUE AND "pages_domains"."auto_ssl_failed" = FALSE AND "pages_domains"."certificate_valid_not_after" IS NULL)
UNION
(SELECT "pages_domains".* FROM "pages_domains" WHERE "pages_domains"."auto_ssl_enabled" = TRUE AND "pages_domains"."auto_ssl_failed" = FALSE AND "pages_domains"."certificate_valid_not_after" < '2020-05-06 10:15:28.921775')) pages_domains;

 HashAggregate  (cost=18267.90..18378.26 rows=11036 width=249) (actual time=1688.487..1689.469 rows=1450 loops=1)
   Group Key: pages_domains.id, pages_domains.project_id, pages_domains.certificate, pages_domains.encrypted_key, pages_domains.encrypted_key_iv, pages_domains.encrypted_key_salt, pages_domains.domain, pages_domains.verified_at, pages_domains.verification_code, pages_domains.enabled_until, pages_domains.remove_at, pages_domains.auto_ssl_enabled, pages_domains.certificate_valid_not_before, pages_domains.certificate_valid_not_after, pages_domains.certificate_source, pages_domains.wildcard, pages_domains.usage, pages_domains.scope, pages_domains.auto_ssl_failed
   Buffers: shared hit=5434 read=1610 dirtied=9
   I/O Timings: read=1647.138
   ->  Append  (cost=0.28..17743.69 rows=11036 width=249) (actual time=0.232..5.752 rows=2456 loops=1)
         Buffers: shared hit=1581 read=11 dirtied=9
         I/O Timings: read=0.566
         ->  Index Scan using index_pages_domains_need_auto_ssl_renewal_user_provided on public.pages_domains  (cost=0.28..9574.10 rows=6378 width=197) (actual time=0.231..1.862 rows=1011 loops=1)
               Buffers: shared hit=752 read=4 dirtied=9
               I/O Timings: read=0.241
         ->  Index Scan using index_pages_domains_need_auto_ssl_renewal_valid_not_after on public.pages_domains pages_domains_1  (cost=0.29..6299.10 rows=3745 width=197) (actual time=0.110..1.026 rows=973 loops=1)
               Index Cond: (pages_domains_1.certificate_valid_not_after IS NULL)
               Buffers: shared hit=405 read=5
               I/O Timings: read=0.222
         ->  Index Scan using index_pages_domains_need_auto_ssl_renewal_valid_not_after on public.pages_domains pages_domains_2  (cost=0.29..1760.13 rows=913 width=197) (actual time=0.048..2.328 rows=472 loops=1)
               Index Cond: (pages_domains_2.certificate_valid_not_after < '2020-05-06 10:15:28.921775+00'::timestamp with time zone)
               Buffers: shared hit=424 read=2
               I/O Timings: read=0.103

Migrations:

== 20200403132349 RemoveOldIndexPagesDomainsNeedAutoSslRenewal: migrating =====
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:pages_domains, [:certificate_source, :certificate_valid_not_after], {:where=>"auto_ssl_enabled = true", :name=>"index_pages_domains_need_auto_ssl_renewal", :algorithm=>:concurrently})
   -> 0.0050s
-- execute("SET statement_timeout TO 0")
   -> 0.0002s
-- remove_index(:pages_domains, {:where=>"auto_ssl_enabled = true", :name=>"index_pages_domains_need_auto_ssl_renewal", :algorithm=>:concurrently, :column=>[:certificate_source, :certificate_valid_not_after]})
   -> 0.0144s
-- execute("RESET ALL")
   -> 0.0002s
== 20200403132349 RemoveOldIndexPagesDomainsNeedAutoSslRenewal: migrated (0.0200s) 

== 20200406095930 AddNeedsSslRenewalUserProvidedPagesDomainsIndex: migrating ==
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:pages_domains, :id, {:where=>"auto_ssl_enabled = true AND auto_ssl_failed = false AND certificate_source = 0", :name=>"index_pages_domains_need_auto_ssl_renewal_user_provided", :algorithm=>:concurrently})
   -> 0.0036s
-- execute("SET statement_timeout TO 0")
   -> 0.0002s
-- add_index(:pages_domains, :id, {:where=>"auto_ssl_enabled = true AND auto_ssl_failed = false AND certificate_source = 0", :name=>"index_pages_domains_need_auto_ssl_renewal_user_provided", :algorithm=>:concurrently})
   -> 0.0034s
-- execute("RESET ALL")
   -> 0.0002s
== 20200406095930 AddNeedsSslRenewalUserProvidedPagesDomainsIndex: migrated (0.0075s) 

== 20200406100909 AddNeedsSslRenewalValidNotAfterPagesDomainsIndex: migrating =
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:pages_domains, :certificate_valid_not_after, {:where=>"auto_ssl_enabled = true AND auto_ssl_failed = false", :name=>"index_pages_domains_need_auto_ssl_renewal_valid_not_after", :algorithm=>:concurrently})
   -> 0.0038s
-- execute("SET statement_timeout TO 0")
   -> 0.0002s
-- add_index(:pages_domains, :certificate_valid_not_after, {:where=>"auto_ssl_enabled = true AND auto_ssl_failed = false", :name=>"index_pages_domains_need_auto_ssl_renewal_valid_not_after", :algorithm=>:concurrently})
   -> 0.0021s
-- execute("RESET ALL")
   -> 0.0002s
== 20200406100909 AddNeedsSslRenewalValidNotAfterPagesDomainsIndex: migrated (0.0065s)

Rollback:

== 20200406100909 AddNeedsSslRenewalValidNotAfterPagesDomainsIndex: reverting =
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:pages_domains, :certificate_valid_not_after, {:where=>"auto_ssl_enabled = true AND auto_ssl_failed = false", :name=>"index_pages_domains_need_auto_ssl_renewal_valid_not_after", :algorithm=>:concurrently})
   -> 0.0039s
-- execute("SET statement_timeout TO 0")
   -> 0.0001s
-- remove_index(:pages_domains, {:where=>"auto_ssl_enabled = true AND auto_ssl_failed = false", :name=>"index_pages_domains_need_auto_ssl_renewal_valid_not_after", :algorithm=>:concurrently, :column=>:certificate_valid_not_after})
   -> 0.0059s
-- execute("RESET ALL")
   -> 0.0001s
== 20200406100909 AddNeedsSslRenewalValidNotAfterPagesDomainsIndex: reverted (0.0102s) 

== 20200406095930 AddNeedsSslRenewalUserProvidedPagesDomainsIndex: reverting ==
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:pages_domains, :id, {:where=>"auto_ssl_enabled = true AND auto_ssl_failed = false AND certificate_source = 0", :name=>"index_pages_domains_need_auto_ssl_renewal_user_provided", :algorithm=>:concurrently})
   -> 0.0031s
-- execute("SET statement_timeout TO 0")
   -> 0.0002s
-- remove_index(:pages_domains, {:where=>"auto_ssl_enabled = true AND auto_ssl_failed = false AND certificate_source = 0", :name=>"index_pages_domains_need_auto_ssl_renewal_user_provided", :algorithm=>:concurrently, :column=>:id})
   -> 0.0054s
-- execute("RESET ALL")
   -> 0.0002s
== 20200406095930 AddNeedsSslRenewalUserProvidedPagesDomainsIndex: reverted (0.0089s) 

== 20200403132349 RemoveOldIndexPagesDomainsNeedAutoSslRenewal: reverting =====
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:pages_domains, [:certificate_source, :certificate_valid_not_after], {:where=>"auto_ssl_enabled = true", :name=>"index_pages_domains_need_auto_ssl_renewal", :algorithm=>:concurrently})
   -> 0.0052s
-- execute("SET statement_timeout TO 0")
   -> 0.0002s
-- add_index(:pages_domains, [:certificate_source, :certificate_valid_not_after], {:where=>"auto_ssl_enabled = true", :name=>"index_pages_domains_need_auto_ssl_renewal", :algorithm=>:concurrently})
   -> 0.0168s
-- execute("RESET ALL")
   -> 0.0003s
== 20200403132349 RemoveOldIndexPagesDomainsNeedAutoSslRenewal: reverted (0.0227s)

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
Tested in all supported browsers
Informed Infrastructure department of a default or new setting change, if applicable per definition of done

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

Label as security and @ mention @gitlab-com/gl-security/appsec
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
Security reports checked/validated by a reviewer from the AppSec team

Edited Apr 22, 2020 by Grzegorz Bizon

Enable Let's Encrypt error handling for all pages domains