zrhoffman commented on code in PR #7764:
URL: https://github.com/apache/trafficcontrol/pull/7764#discussion_r1312208424


##########
traffic_ops/app/db/create_tables.sql:
##########
@@ -841,7 +835,6 @@ CREATE TABLE IF NOT EXISTS deliveryservice_tmuser (
     CONSTRAINT idx_89525_primary PRIMARY KEY (deliveryservice, tm_user_id)
 );
 
-
 ALTER TABLE deliveryservice_tmuser OWNER TO traffic_ops;

Review Comment:
   The `deliveryservice_tmuser` table was removed in 
`2022031612431414_remove_unused_ds_tmuser_table.up.sql`:
   
   
https://github.com/apache/trafficcontrol/blob/2257ddb1a49c0b157206a92e753a01ce46eab165/traffic_ops/app/db/migrations/2022031612431414_remove_unused_ds_tmuser_table.up.sql#L18



##########
traffic_ops/app/db/create_tables.sql:
##########
@@ -3811,6 +3734,460 @@ 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_total_limits; Type: TABLE; Schema: public; Owner: traffic_ops
+--
+
+CREATE TABLE IF NOT EXISTS cdni_total_limits (
+    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_total_limits OWNER TO traffic_ops;
+
+--
+-- Name: cdni_host_limits; Type: TABLE; Schema: public; Owner: traffic_ops
+--
+
+CREATE TABLE IF NOT EXISTS cdni_host_limits (
+    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,
+    host text NOT NULL,
+    last_updated timestamp with time zone DEFAULT now() NOT NULL
+);
+
+ALTER TABLE cdni_host_limits OWNER TO traffic_ops;

Review Comment:
   The `cdni_host_limits` table was dropped in  
`2022050410412200_cdni_capacity_limit_with_scopes.up.sql`:
   
   
https://github.com/apache/trafficcontrol/blob/2257ddb1a49c0b157206a92e753a01ce46eab165/traffic_ops/app/db/migrations/2022050410412200_cdni_capacity_limit_with_scopes.up.sql#L76



##########
traffic_ops/app/db/migrations/2022040623082100_server_config_update.up.sql:
##########
@@ -1,28 +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 
under
- * the License.
- */
-
--- Add columns, timestamp with time zone, nullable, default null
-ALTER TABLE public.server
-ADD COLUMN IF NOT EXISTS config_update_time TIMESTAMPTZ NOT NULL DEFAULT 
TIMESTAMP 'epoch',
-ADD COLUMN IF NOT EXISTS config_apply_time TIMESTAMPTZ NOT NULL DEFAULT 
TIMESTAMP 'epoch', 
-ADD COLUMN IF NOT EXISTS revalidate_update_time TIMESTAMPTZ NOT NULL DEFAULT 
TIMESTAMP 'epoch', 
-ADD COLUMN IF NOT EXISTS revalidate_apply_time TIMESTAMPTZ NOT NULL DEFAULT 
TIMESTAMP 'epoch';

Review Comment:
   These columns need to be added to `create_tables.sql`.



##########
traffic_ops/app/db/migrations/2022021611354000_add_user_to_ucdn_table.up.sql:
##########
@@ -1,18 +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 
under
- * the License.
- */
-
-ALTER TABLE tm_user ADD COLUMN IF NOT EXISTS ucdn text NOT NULL DEFAULT '';

Review Comment:
   The `ucdn` column needs to be added to the `tm_user` `CREATE TABLE` 
statement in `create_tables.sql`.



##########
traffic_ops/app/db/create_tables.sql:
##########
@@ -3811,6 +3734,460 @@ 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_total_limits; Type: TABLE; Schema: public; Owner: traffic_ops
+--
+
+CREATE TABLE IF NOT EXISTS cdni_total_limits (
+    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_total_limits OWNER TO traffic_ops;

Review Comment:
   The `cdni_total_limits` table was dropped in  
`2022050410412200_cdni_capacity_limit_with_scopes.up.sql`:
   
   
https://github.com/apache/trafficcontrol/blob/2257ddb1a49c0b157206a92e753a01ce46eab165/traffic_ops/app/db/migrations/2022050410412200_cdni_capacity_limit_with_scopes.up.sql#L75



##########
traffic_ops/app/db/squash_migrations.sh:
##########
@@ -25,6 +25,7 @@ last_squashed_migration="$(<<<"$migrations_to_squash" tail 
-n1)"
 last_squashed_migration_timestamp="$(<<<"$last_squashed_migration" sed -E 
's|migrations/([0-9]+).*|\1|')"
 first_migration="$(ls migrations/*.sql | grep -A1 
"/${last_squashed_migration_timestamp}_" | tail -n1)"
 first_migration_timestamp="$(<<<"$first_migration" sed -E 
's|migrations/([0-9]+).*|\1|')"
+# If running on mac, you need BSD version of sed. So, change -i to -i.bak for 
the below three sed statements.
 sed -i '/^--/,$d' create_tables.sql # keeps the Apache License 2.0 header

Review Comment:
   Just modify the `sed` command, the modified command works on both the GNU 
and BSD versions of `sed`.



##########
traffic_ops/app/db/migrations/2022040623082100_server_config_update.up.sql:
##########
@@ -1,28 +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 
under
- * the License.
- */
-
--- Add columns, timestamp with time zone, nullable, default null
-ALTER TABLE public.server
-ADD COLUMN IF NOT EXISTS config_update_time TIMESTAMPTZ NOT NULL DEFAULT 
TIMESTAMP 'epoch',
-ADD COLUMN IF NOT EXISTS config_apply_time TIMESTAMPTZ NOT NULL DEFAULT 
TIMESTAMP 'epoch', 
-ADD COLUMN IF NOT EXISTS revalidate_update_time TIMESTAMPTZ NOT NULL DEFAULT 
TIMESTAMP 'epoch', 
-ADD COLUMN IF NOT EXISTS revalidate_apply_time TIMESTAMPTZ NOT NULL DEFAULT 
TIMESTAMP 'epoch';
-
--- Drop previous columns
-ALTER TABLE public.server 
-DROP COLUMN IF EXISTS upd_pending, 
-DROP COLUMN IF EXISTS reval_pending;

Review Comment:
   These columns need to be removed from `create_tables.sql`.



##########
traffic_ops/app/db/seeds.sql:
##########
@@ -97,6 +144,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 >= 30
+    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:
   `INSERT` statements belong in `seeds.sql`. Also, this results in the `admin` 
role and the `router` role gaining perms they did not previously have (role `1` 
is `trouter`, role `2` is `admin`):
   
   ```diff
   --- apache.sql       2023-08-31 14:42:08.747837019 -0600
   +++ 7764.sql 2023-08-31 14:42:41.985179003 -0600
   @@ -3022,6 +3091,9 @@
    6   DELIVERY-SERVICE-SAFE:UPDATE    A_TIMESTAMP
    7   DELIVERY-SERVICE-SAFE:UPDATE    A_TIMESTAMP
    8   DELIVERY-SERVICE-SAFE:UPDATE    A_TIMESTAMP
   +2   DNS-SEC:READ    A_TIMESTAMP
   +2   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/squash_migrations.sh:
##########
@@ -25,6 +25,7 @@ last_squashed_migration="$(<<<"$migrations_to_squash" tail 
-n1)"
 last_squashed_migration_timestamp="$(<<<"$last_squashed_migration" sed -E 
's|migrations/([0-9]+).*|\1|')"
 first_migration="$(ls migrations/*.sql | grep -A1 
"/${last_squashed_migration_timestamp}_" | tail -n1)"
 first_migration_timestamp="$(<<<"$first_migration" sed -E 
's|migrations/([0-9]+).*|\1|')"
+# If running on mac, you need BSD version of sed. So, change -i to -i.bak for 
the below three sed statements.
 sed -i '/^--/,$d' create_tables.sql # keeps the Apache License 2.0 header
 sed -Ei "s|(LastSquashedMigrationTimestamp\s+uint\s+= 
).*|\1${last_squashed_migration_timestamp} // ${last_squashed_migration}|" 
admin.go
 sed -Ei "s|(FirstMigrationTimestamp\s+uint\s+= 
).*|\1${first_migration_timestamp} // ${first_migration}|" admin.go

Review Comment:
   Might as well remove these lines while you're in here



-- 
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: issues-unsubscr...@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to