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


##########
traffic_ops/app/db/migrations/2021110210085600_add_permissions.up.sql:
##########
@@ -1,164 +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 built-in roles
-INSERT INTO public.role (name, description, priv_level) VALUES ('operations', 
'Has all reads and most write capabilities', 20) ON CONFLICT (name) DO NOTHING;
-INSERT INTO public.role (name, description, priv_level) VALUES ('read-only', 
'Has access to all read capabilities', 10) ON CONFLICT (name) DO NOTHING;
-INSERT INTO public.role (name, description, priv_level) values ('disallowed', 
'Block all access', 0) ON CONFLICT (name) DO NOTHING;
-INSERT INTO public.role (name, description, priv_level) VALUES 
('portal','Portal User', 2) ON CONFLICT DO NOTHING;
-INSERT INTO public.role (name, description, priv_level) VALUES 
('steering','Steering User', 15) ON CONFLICT DO NOTHING;
-INSERT INTO public.role (name, description, priv_level) VALUES 
('federation','Role for Secondary CZF', 15) ON CONFLICT DO NOTHING;

Review Comment:
   Removing this stuff means that `seeds.sql` needs to be updated:
   
https://github.com/apache/trafficcontrol/blob/8e75d3ee76c5fe637221ff47efe8ab12402f5870/traffic_ops/app/db/seeds.sql#L76-L81
   
   The `ON CONFLICT DO NOTHING`s make it fail silently, which seems bad.



##########
traffic_ops/app/db/migrations/2021110210085600_add_permissions.up.sql:
##########
@@ -1,164 +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 built-in roles
-INSERT INTO public.role (name, description, priv_level) VALUES ('operations', 
'Has all reads and most write capabilities', 20) ON CONFLICT (name) DO NOTHING;
-INSERT INTO public.role (name, description, priv_level) VALUES ('read-only', 
'Has access to all read capabilities', 10) ON CONFLICT (name) DO NOTHING;
-INSERT INTO public.role (name, description, priv_level) values ('disallowed', 
'Block all access', 0) ON CONFLICT (name) DO NOTHING;
-INSERT INTO public.role (name, description, priv_level) VALUES 
('portal','Portal User', 2) ON CONFLICT DO NOTHING;
-INSERT INTO public.role (name, description, priv_level) VALUES 
('steering','Steering User', 15) ON CONFLICT DO NOTHING;
-INSERT INTO public.role (name, description, priv_level) VALUES 
('federation','Role for Secondary CZF', 15) ON CONFLICT DO NOTHING;
-
--- add permissions to roles in order of decreasing priv level
-INSERT INTO public.role_capability SELECT id, perm FROM public.role CROSS JOIN 
(VALUES
-('ASN:CREATE'),
-('ASN:DELETE'),
-('ASN:UPDATE'),
-('CACHE-GROUP:CREATE'),

Review Comment:
   Seeding roles in `seeds.sql` fails, so that part need needs to be updated, 
too
   
   
https://github.com/apache/trafficcontrol/blob/8e75d3ee76c5fe637221ff47efe8ab12402f5870/traffic_ops/app/db/seeds.sql#L162-L170=



##########
traffic_ops/app/db/trafficvault/create_tables.sql:
##########
@@ -76,6 +76,8 @@ CREATE TABLE IF NOT EXISTS sslkey (
     deliveryservice text NOT NULL,
     cdn text NOT NULL,
     version text NOT NULL,
+    provider text NOT NULL,
+    expiration timestamp with time zone NOT NULL DEFAULT now(),
     last_updated timestamp with time zone DEFAULT now() NOT NULL

Review Comment:
   `last_updated` should come before `provider` or `expiration`:
   
   ```diff
   --- apache.sql       2022-05-16 12:55:10.975056760 -0600
   +++ 6802.sql 2022-05-16 12:55:23.841937863 -0600
   @@ -58,9 +58,9 @@
        deliveryservice text NOT NULL,
        cdn text NOT NULL,
        version text NOT NULL,
   -    last_updated timestamp with time zone DEFAULT now() NOT NULL,
        provider text NOT NULL,
   -    expiration timestamp with time zone DEFAULT now() NOT NULL
   +    expiration timestamp with time zone DEFAULT now() NOT NULL,
   +    last_updated timestamp with time zone DEFAULT now() NOT NULL
    );
   ```



##########
traffic_ops/app/db/create_tables.sql:
##########
@@ -462,6 +534,54 @@ ALTER TABLE cdn_id_seq OWNER TO traffic_ops;
 
 ALTER SEQUENCE cdn_id_seq OWNED BY cdn.id;
 
+--
+-- Name: cdn_lock; Type: TABLE; Schema: public; Owner: traffic_ops
+--
+
+CREATE TABLE IF NOT EXISTS public.cdn_lock (
+    username text NOT NULL,
+    cdn text NOT NULL,
+    message text,
+    soft boolean NOT NULL DEFAULT TRUE,
+    last_updated timestamp with time zone DEFAULT now() NOT NULL,
+    CONSTRAINT pk_cdn_lock PRIMARY KEY ("cdn")
+);
+
+
+ALTER TABLE cdn_lock OWNER TO traffic_ops;
+
+--
+-- Name: cdn_notification; Type: TABLE; Schema: public; Owner: traffic_ops
+
+CREATE TABLE cdn_notification (
+    id bigint NOT NULL,
+    cdn text NOT NULL,
+    "user" text NOT NULL,
+    notification text,
+    last_updated timestamp with time zone DEFAULT now() NOT NULL,
+    CONSTRAINT pk_cdn_notification PRIMARY KEY (id)
+);
+
+--
+-- Name: cdn_notification_id_seq; Type: SEQUENCE; Schema: public; Owner: 
traffic_ops
+--
+
+CREATE SEQUENCE IF NOT EXISTS cdn_notification_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+
+ALTER TABLE cdn_notification_id_seq OWNER TO traffic_ops;
+
+--
+-- Name: cdn_notification_id_seq; Type: SEQUENCE OWNED BY; Schema: public; 
Owner: traffic_ops
+--
+
+ALTER SEQUENCE cdn_notification_id_seq OWNED BY cdn.id;

Review Comment:
   `cdn_notification_id_seq` used to be owned by `cdn_notification.id`:
   
   ```diff
   @@ -648,8 +633,23 @@
    -- Name: cdn_notification_id_seq; Type: SEQUENCE OWNED BY; Schema: public; 
Owner: traffic_ops
    --
    
   -ALTER SEQUENCE public.cdn_notification_id_seq OWNED BY 
public.cdn_notification.id;
   +ALTER SEQUENCE public.cdn_notification_id_seq OWNED BY public.cdn.id;
   ```



##########
traffic_ops/app/db/create_tables.sql:
##########
@@ -462,6 +534,54 @@ ALTER TABLE cdn_id_seq OWNER TO traffic_ops;
 
 ALTER SEQUENCE cdn_id_seq OWNED BY cdn.id;
 
+--
+-- Name: cdn_lock; Type: TABLE; Schema: public; Owner: traffic_ops
+--
+
+CREATE TABLE IF NOT EXISTS public.cdn_lock (
+    username text NOT NULL,
+    cdn text NOT NULL,
+    message text,
+    soft boolean NOT NULL DEFAULT TRUE,
+    last_updated timestamp with time zone DEFAULT now() NOT NULL,
+    CONSTRAINT pk_cdn_lock PRIMARY KEY ("cdn")
+);
+
+
+ALTER TABLE cdn_lock OWNER TO traffic_ops;
+
+--
+-- Name: cdn_notification; Type: TABLE; Schema: public; Owner: traffic_ops
+
+CREATE TABLE cdn_notification (
+    id bigint NOT NULL,
+    cdn text NOT NULL,
+    "user" text NOT NULL,
+    notification text,
+    last_updated timestamp with time zone DEFAULT now() NOT NULL,
+    CONSTRAINT pk_cdn_notification PRIMARY KEY (id)

Review Comment:
   A db dump after running `create_tables.sql` and up migrations shows the 
`cdn_notifications` primary key used to be named `cdn_notification_pkey`:
   `
   ```diff
   @@ -3717,14 +3433,6 @@
    
    
    --
   --- Name: cdn_notification cdn_notification_pkey; Type: CONSTRAINT; Schema: 
public; Owner: traffic_ops
   ---
   -
   -ALTER TABLE ONLY public.cdn_notification
   -    ADD CONSTRAINT cdn_notification_pkey PRIMARY KEY (id);
   -
   -
   ---
    -- Name: coordinate coordinate_name_key; Type: CONSTRAINT; Schema: public; 
Owner: traffic_ops
    --
    
   @@ -4093,6 +3801,14 @@
    
    
    --
   +-- Name: cdn_notification pk_cdn_notification; Type: CONSTRAINT; Schema: 
public; Owner: traffic_ops
   +--
   +
   +ALTER TABLE ONLY public.cdn_notification
   +    ADD CONSTRAINT pk_cdn_notification PRIMARY KEY (id);
   +
   +
   +--
    -- Name: cdni_capabilities pk_cdni_capabilities; Type: CONSTRAINT; Schema: 
public; Owner: traffic_ops
    --
   ```



-- 
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]

Reply via email to