zrhoffman commented on code in PR #7764: URL: https://github.com/apache/trafficcontrol/pull/7764#discussion_r1312384741
########## CHANGELOG.md: ########## @@ -79,31 +80,34 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#7765](https://github.com/apache/trafficcontrol/pull/7765) *Traffic Stats* now uses Traffic Ops APIv5 ### Fixed +- [#7764](https://github.com/apache/trafficcontrol/pull/7764) *Traffic Ops* Collapsed DB migrations - [#7767](https://github.com/apache/trafficcontrol/pull/7767) *Traffic Ops* Fixed ASN update logic for APIv5 - [RFC3339](https://github.com/apache/trafficcontrol/issues/5911) - - [#7759](https://github.com/apache/trafficcontrol/pull/7759) *Traffic Ops* Fixed `/profiles/{{ID}}/parameters` and `profiles/name/{{name}}/parameters` v5 APIs to respond with `RFC3339` timestamps. Review Comment: Why is there such a big diff in the changelog? ########## traffic_ops/app/db/seeds.sql: ########## @@ -97,6 +103,23 @@ FROM public.role WHERE "name" in ('operations', 'read-only', 'portal', 'federation', 'steering') ON CONFLICT DO NOTHING; +INSERT INTO public.role_capability +SELECT id, perm FROM public.role +CROSS JOIN ( + VALUES ('DNS-SEC:READ'), ('DNS-SEC:DELETE') +) AS perms(perm) +WHERE "priv_level" < 20 AND "priv_level" > 0 + ON CONFLICT DO NOTHING; Review Comment: This grants `DNS-SEC:READ` and `DNS-SEC:DELETE` to a bunch of roles that did not have it before: trouter (`1`), read-only (`4`), portal (`6`), steering (`7`), and federation (`8`): ```diff --- apache.sql 2023-08-31 17:46:13.333267263 -0600 +++ 7764.sql 2023-08-31 17:46:46.260625355 -0600 @@ -3022,6 +3022,16 @@ 6 DELIVERY-SERVICE-SAFE:UPDATE A_TIMESTAMP 7 DELIVERY-SERVICE-SAFE:UPDATE A_TIMESTAMP 8 DELIVERY-SERVICE-SAFE:UPDATE A_TIMESTAMP +1 DNS-SEC:DELETE A_TIMESTAMP +4 DNS-SEC:READ A_TIMESTAMP +4 DNS-SEC:DELETE A_TIMESTAMP +6 DNS-SEC:READ A_TIMESTAMP +6 DNS-SEC:DELETE A_TIMESTAMP +7 DNS-SEC:READ A_TIMESTAMP +7 DNS-SEC:DELETE A_TIMESTAMP +8 DNS-SEC:READ A_TIMESTAMP +8 DNS-SEC:DELETE A_TIMESTAMP +1 DELIVERY-SERVICE-SAFE:UPDATE A_TIMESTAMP 3 ACME:READ A_TIMESTAMP 4 ACME:READ A_TIMESTAMP 6 ACME:READ A_TIMESTAMP ``` ########## traffic_ops/app/db/migrations/2022041410185700_add_server_profile_table.up.sql: ########## @@ -1,38 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with this - * work for additional information regarding copyright ownership. The ASF - * licenses this file to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations undera - * the License. - */ - -CREATE TABLE IF NOT EXISTS public.server_profile ( - server bigint NOT NULL, - profile_name text NOT NULL, - priority int NOT NULL CHECK (priority >= 0), - CONSTRAINT pk_server_profile PRIMARY KEY (profile_name, server), - CONSTRAINT fk_server_id FOREIGN KEY (server) REFERENCES public.server(id) ON DELETE CASCADE ON UPDATE CASCADE, - CONSTRAINT fk_server_profile_name_profile FOREIGN KEY (profile_name) REFERENCES public.profile(name) ON UPDATE CASCADE ON DELETE RESTRICT - ); - -INSERT into public.server_profile(server, profile_name, priority) -SELECT s.id, p.name, 0 -FROM public.server AS s - JOIN public.profile p ON p.id=s.profile; - -DROP TRIGGER IF EXISTS before_update_ip_address_trigger on ip_address; - -DROP TRIGGER IF EXISTS before_create_ip_address_trigger on ip_address; - -DROP TRIGGER IF EXISTS before_update_server_trigger ON server; - -DROP TRIGGER IF EXISTS before_create_server_trigger ON server; Review Comment: These triggers are created in `create_tables.sql` and those queries need to be removed. ########## traffic_ops/app/db/seeds.sql: ########## @@ -97,6 +103,23 @@ FROM public.role WHERE "name" in ('operations', 'read-only', 'portal', 'federation', 'steering') ON CONFLICT DO NOTHING; +INSERT INTO public.role_capability +SELECT id, perm FROM public.role +CROSS JOIN ( + VALUES ('DNS-SEC:READ'), ('DNS-SEC:DELETE') +) AS perms(perm) +WHERE "priv_level" < 20 AND "priv_level" > 0 + ON CONFLICT DO NOTHING; + +INSERT INTO public.role_capability (role_id, cap_name) +SELECT id, perm +FROM public.role +CROSS JOIN ( + VALUES ('DELIVERY-SERVICE-SAFE:UPDATE') +) AS perms(perm) +WHERE "priv_level" < 20 AND "priv_level" > 0 + ON CONFLICT DO NOTHING; Review Comment: This grants `DELIVERY-SERVICE-SAFE:UPDATE` to the `trouter` user, but `trouter` should not have `DELIVERY-SERVICE-SAFE:UPDATE`. ########## traffic_ops/app/db/create_tables.sql: ########## @@ -1492,8 +1443,11 @@ CREATE TABLE IF NOT EXISTS server ( guid text, last_updated timestamp with time zone NOT NULL DEFAULT now(), https_port bigint, - reval_pending boolean NOT NULL DEFAULT FALSE, status_last_updated timestamp with time zone, + config_update_time timestamp NOT NULL DEFAULT TIMESTAMP 'epoch', + config_apply_time timestamp NOT NULL DEFAULT TIMESTAMP 'epoch', + revalidate_update_time timestamp NOT NULL DEFAULT TIMESTAMP 'epoch', + revalidate_apply_time timestamp NOT NULL DEFAULT TIMESTAMP 'epoch', Review Comment: These lines produce different columns than the migration did (`with time zone DEFAULT` vs `without time zone DEFAULT`): ```diff --- apache.sql 2023-08-31 17:31:58.303674009 -0600 +++ 7764.sql 2023-08-31 17:32:30.757614383 -0600 @@ -1760,10 +1760,10 @@ last_updated timestamp with time zone DEFAULT now() NOT NULL, https_port bigint, status_last_updated timestamp with time zone, - config_update_time timestamp with time zone DEFAULT '1970-01-01 00:00:00'::timestamp without time zone NOT NULL, - config_apply_time timestamp with time zone DEFAULT '1970-01-01 00:00:00'::timestamp without time zone NOT NULL, - revalidate_update_time timestamp with time zone DEFAULT '1970-01-01 00:00:00'::timestamp without time zone NOT NULL, - revalidate_apply_time timestamp with time zone DEFAULT '1970-01-01 00:00:00'::timestamp without time zone NOT NULL + config_update_time timestamp without time zone DEFAULT '1970-01-01 00:00:00'::timestamp without time zone NOT NULL, + config_apply_time timestamp without time zone DEFAULT '1970-01-01 00:00:00'::timestamp without time zone NOT NULL, + revalidate_update_time timestamp without time zone DEFAULT '1970-01-01 00:00:00'::timestamp without time zone NOT NULL, + revalidate_apply_time timestamp without time zone DEFAULT '1970-01-01 00:00:00'::timestamp without time zone NOT NULL ); ``` The migration, for reference: https://github.com/apache/trafficcontrol/blob/d385dcb04ebce7792d4b13370b98c604b59408f5/traffic_ops/app/db/migrations/2022040623082100_server_config_update.up.sql#L20-L23 ########## traffic_ops/app/db/create_tables.sql: ########## @@ -3811,6 +3696,385 @@ REVOKE ALL ON SCHEMA public FROM traffic_ops; GRANT ALL ON SCHEMA public TO traffic_ops; GRANT ALL ON SCHEMA public TO PUBLIC; +-- +-- Name: cdni_capabilities; Type: TABLE; Schema: public; Owner: traffic_ops +-- + +CREATE TABLE cdni_capabilities ( + id bigint NOT NULL, + type text NOT NULL, + ucdn text NOT NULL, + last_updated timestamp with time zone DEFAULT now() NOT NULL +); + +ALTER TABLE cdni_capabilities OWNER TO traffic_ops; + +-- +-- Name: cdni_capabilities_id_seq; Type: SEQUENCE; Schema: public; Owner: traffic_ops +-- + +CREATE SEQUENCE cdni_capabilities_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER TABLE cdni_capabilities_id_seq OWNER TO traffic_ops; + +-- +-- Name: cdni_capabilities_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: traffic_ops +-- + +ALTER SEQUENCE cdni_capabilities_id_seq OWNED BY cdni_capabilities.id; + +-- +-- Name: cdni_footprints; Type: TABLE; Schema: public; Owner: traffic_ops +-- + +CREATE TABLE cdni_footprints ( + id bigint NOT NULL, + footprint_type text NOT NULL, + footprint_value text[] NOT NULL, + ucdn text NOT NULL, + capability_id bigint NOT NULL, + last_updated timestamp with time zone DEFAULT now() NOT NULL +); + +ALTER TABLE cdni_footprints OWNER TO traffic_ops; + +-- +-- Name: cdni_footprints_id_seq; Type: SEQUENCE; Schema: public; Owner: traffic_ops +-- + +CREATE SEQUENCE cdni_footprints_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER TABLE cdni_footprints_id_seq OWNER TO traffic_ops; + +-- +-- Name: cdni_footprints_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: traffic_ops +-- + +ALTER SEQUENCE cdni_footprints_id_seq OWNED BY cdni_footprints.id; + +-- +-- Name: cdni_limits; Type: TABLE; Schema: public; Owner: traffic_ops +-- + +CREATE TABLE cdni_limits ( + id bigint NOT NULL, + limit_id text NOT NULL, + scope_type text, + scope_value text[], + limit_type text NOT NULL, + maximum_hard bigint NOT NULL, + maximum_soft bigint NOT NULL, + telemetry_id text NOT NULL, + telemetry_metric text NOT NULL, + capability_id bigint NOT NULL, + last_updated timestamp with time zone DEFAULT now() NOT NULL +); + +ALTER TABLE cdni_limits OWNER TO traffic_ops; + +-- +-- Name: cdni_limits_id_seq; Type: SEQUENCE; Schema: public; Owner: traffic_ops +-- + +CREATE SEQUENCE cdni_limits_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER TABLE cdni_limits_id_seq OWNER TO traffic_ops; + +-- +-- Name: cdni_limits_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: traffic_ops +-- + +ALTER SEQUENCE cdni_limits_id_seq OWNED BY cdni_limits.id; + +-- +-- Name: cdni_capabilities; Type: SEQUENCE OWNED BY; Schema: public; Owner: traffic_ops +-- + +CREATE TABLE IF NOT EXISTS cdni_capabilities ( + id bigserial NOT NULL, + type text NOT NULL, + ucdn text NOT NULL, + last_updated timestamp with time zone DEFAULT now() NOT NULL +); + +ALTER TABLE cdni_capabilities OWNER TO traffic_ops; + +-- +-- Name: cdni_footprints; Type: TABLE; Schema: public; Owner: traffic_ops +-- + +CREATE TABLE IF NOT EXISTS cdni_footprints ( + id bigserial NOT NULL, + footprint_type text NOT NULL, + footprint_value text[] NOT NULL, + ucdn text NOT NULL, + capability_id bigint NOT NULL, + last_updated timestamp with time zone DEFAULT now() NOT NULL +); + +ALTER TABLE cdni_footprints OWNER TO traffic_ops; + +-- +-- Name: cdni_telemetry; Type: TABLE; Schema: public; Owner: traffic_ops +-- + +CREATE TABLE cdni_telemetry ( + id text NOT NULL, + type text NOT NULL, + capability_id bigint NOT NULL, + last_updated timestamp with time zone DEFAULT now() NOT NULL, + configuration_url text DEFAULT ''::text +); + +ALTER TABLE cdni_telemetry OWNER TO traffic_ops; + +-- +-- Name: cdni_telemetry_metrics; Type: TABLE; Schema: public; Owner: traffic_ops +-- + +CREATE TABLE cdni_telemetry_metrics ( + name text NOT NULL, + time_granularity bigint NOT NULL, + data_percentile bigint NOT NULL, + latency integer NOT NULL, + telemetry_id text NOT NULL, + last_updated timestamp with time zone DEFAULT now() NOT NULL +); + +ALTER TABLE cdni_telemetry_metrics OWNER TO traffic_ops; + +-- +-- Name: cdni_limits; Type: TABLE; Schema: public; Owner: traffic_ops +-- + +CREATE TABLE IF NOT EXISTS cdni_limits ( + id bigserial NOT NULL, + limit_id text NOT NULL, + scope_type text, + scope_value text[], + limit_type text NOT NULL, + maximum_hard bigint NOT NULL, + maximum_soft bigint NOT NULL, + telemetry_id text NOT NULL, + telemetry_metric text NOT NULL, + capability_id bigint NOT NULL, + last_updated timestamp with time zone DEFAULT now() NOT NULL +); + +ALTER TABLE cdni_limits OWNER TO traffic_ops; + +-- +-- Name: cdn_lock_user; Type: TABLE; Schema: public; Owner: traffic_ops +-- + +CREATE TABLE IF NOT EXISTS cdn_lock_user ( + owner text NOT NULL, + cdn text NOT NULL, + username text NOT NULL +); + +ALTER TABLE cdn_lock_user OWNER TO traffic_ops; + +-- +-- Name: server_profile; Type: TABLE; Schema: public; Owner: traffic_ops +-- + +CREATE TABLE IF NOT EXISTS server_profile ( + server bigint NOT NULL, + profile_name text NOT NULL, + priority int NOT NULL CHECK (priority >= 0) +); + +ALTER TABLE server_profile OWNER TO traffic_ops; + +-- +-- Name: cdni_capability_updates; Type: TABLE; Schema: public; Owner: traffic_ops +-- + +CREATE TABLE IF NOT EXISTS cdni_capability_updates ( + id bigserial NOT NULL, + request_type text NOT NULL, + ucdn text NOT NULL, + host text, + data json NOT NULL, + async_status_id bigint NOT NULL, + last_updated timestamp with time zone DEFAULT now() NOT NULL +); + +ALTER TABLE cdni_capability_updates OWNER TO traffic_ops; + +-- +-- Name: cdni_capabilities id; Type: DEFAULT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdni_capabilities ALTER COLUMN id SET DEFAULT nextval('cdni_capabilities_id_seq'::regclass); + +-- +-- Name: cdni_footprints id; Type: DEFAULT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdni_footprints ALTER COLUMN id SET DEFAULT nextval('cdni_footprints_id_seq'::regclass); + +-- +-- Name: cdni_limits id; Type: DEFAULT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdni_limits ALTER COLUMN id SET DEFAULT nextval('cdni_limits_id_seq'::regclass); + +-- +-- Name: cdn_lock cdn_lock_cdn_username_unique; Type: CONSTRAINT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE cdn_lock ADD CONSTRAINT cdn_lock_cdn_username_unique UNIQUE (username, cdn); + +-- +-- Name: cdni_capabilities pk_cdni_capabilities; Type: CONSTRAINT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdni_capabilities + ADD CONSTRAINT pk_cdni_capabilities PRIMARY KEY (id); + +-- +-- Name: cdni_footprints pk_cdni_footprints; Type: CONSTRAINT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdni_footprints + ADD CONSTRAINT pk_cdni_footprints PRIMARY KEY (id); + +-- +-- Name: cdni_limits pk_cdni_limits; Type: CONSTRAINT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdni_limits + ADD CONSTRAINT pk_cdni_limits PRIMARY KEY (id); + +-- +-- Name: cdni_telemetry pk_cdni_telemetry; Type: CONSTRAINT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdni_telemetry + ADD CONSTRAINT pk_cdni_telemetry PRIMARY KEY (id); + +-- +-- Name: cdni_telemetry_metrics pk_cdni_telemetry_metrics; Type: CONSTRAINT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdni_telemetry_metrics + ADD CONSTRAINT pk_cdni_telemetry_metrics PRIMARY KEY (name); + +-- +-- Name: cdn_lock_user pk_cdn_lock_user; Type: FK CONSTRAINT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdn_lock_user + ADD CONSTRAINT pk_cdn_lock_user PRIMARY KEY (owner, cdn, username); + +-- +-- Name: server_profile pk_server_profile; Type: FK CONSTRAINT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY server_profile + ADD CONSTRAINT pk_server_profile PRIMARY KEY (profile_name, server); + +-- +-- Name: cdni_capability_updates pk_cdni_capability_updates; Type: FK CONSTRAINT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdni_capability_updates + ADD CONSTRAINT pk_cdni_capability_updates PRIMARY KEY (id); + +-- +-- Name: cdni_footprints fk_cdni_footprint_capabilities; Type: FK CONSTRAINT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdni_footprints + ADD CONSTRAINT fk_cdni_footprint_capabilities FOREIGN KEY (capability_id) REFERENCES cdni_capabilities(id) ON UPDATE CASCADE ON DELETE CASCADE; + +-- +-- Name: cdni_limits fk_cdni_limits_capabilities; Type: FK CONSTRAINT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdni_limits + ADD CONSTRAINT fk_cdni_limits_capabilities FOREIGN KEY (capability_id) REFERENCES cdni_capabilities(id) ON UPDATE CASCADE ON DELETE CASCADE; + +-- +-- Name: cdni_limits fk_cdni_limits_telemetry; Type: FK CONSTRAINT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdni_limits + ADD CONSTRAINT fk_cdni_limits_telemetry FOREIGN KEY (telemetry_id) REFERENCES cdni_telemetry(id) ON UPDATE CASCADE ON DELETE CASCADE; + +-- +-- Name: cdni_telemetry fk_cdni_telemetry_capabilities; Type: FK CONSTRAINT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdni_telemetry + ADD CONSTRAINT fk_cdni_telemetry_capabilities FOREIGN KEY (capability_id) REFERENCES cdni_capabilities(id) ON UPDATE CASCADE ON DELETE CASCADE; + +-- +-- Name: cdni_telemetry_metrics fk_cdni_telemetry_metrics_telemetry; Type: FK CONSTRAINT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdni_telemetry_metrics + ADD CONSTRAINT fk_cdni_telemetry_metrics_telemetry FOREIGN KEY (telemetry_id) REFERENCES cdni_telemetry(id) ON UPDATE CASCADE ON DELETE CASCADE; + +-- +-- Name: cdni_telemetry_metrics fk_cdni_total_limits_telemetry; Type: CONSTRAINT; Schema: public; Owner: traffic_ops +-- + +ALTER TABLE ONLY cdni_telemetry_metrics + ADD CONSTRAINT fk_cdni_total_limits_telemetry FOREIGN KEY (telemetry_id) REFERENCES cdni_telemetry(id) ON UPDATE CASCADE ON DELETE CASCADE; Review Comment: Remove this constraint, it did not exist in the original migration. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
