You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2021/05/04 21:22:11 UTC

[GitHub] [trafficcontrol] srijeet0406 opened a new pull request #5812: Implement SSLKeys methods for PostgreSQL Traffic Vault backend

srijeet0406 opened a new pull request #5812:
URL: https://github.com/apache/trafficcontrol/pull/5812


   <!--
   ************ STOP!! ************
   If this Pull Request is intended to fix a security vulnerability, DO NOT submit it! Instead, contact
   the Apache Software Foundation Security Team at security@trafficcontrol.apache.org and follow the
   guidelines at https://www.apache.org/security/ regarding vulnerability disclosure.
   -->
   ## What does this PR (Pull Request) do?
   <!-- Explain the changes you made here. If this fixes an Issue, identify it by
   replacing the text in the checkbox item with the Issue number e.g.
   
   - [x] This PR fixes #9001 OR is not related to any Issue
   
   ^ This will automatically close Issue number 9001 when the Pull Request is
   merged (The '#' is important).
   
   Be sure you check the box properly, see the "The following criteria are ALL
   met by this PR" section for details.
   -->
   
   - [x] This PR is not related to any Issue <!-- You can check for an issue here: https://github.com/apache/trafficcontrol/issues -->
   
   
   ## Which Traffic Control components are affected by this PR?
   <!-- Please delete all components from this list that are NOT affected by this
   Pull Request. Also, feel free to add the name of a tool or script that is
   affected but not on the list.
   
   Additionally, if this Pull Request does NOT affect documentation, please
   explain why documentation is not required. -->
   
   - Traffic Ops
   - Traffic Vault
   
   ## What is the best way to verify this PR?
   <!-- Please include here ALL the steps necessary to test your Pull Request. If
   it includes tests (and most should), outline here the steps needed to run the
   tests. If not, lay out the manual testing procedure and please explain why
   tests are unnecessary for this Pull Request. -->
   
   Setup TO to be configured to use `postgres` as the traffic vault backend.
   Make sure you have the `tv_development` and `tv_test` databases and their corresponding tables setup correctly.
   Try generating SSL keys for deliveryservices, and GETting them and make sure they work finr.
   Check the same in the database.
   Repeat for CDN SSL keys.
   
   Make sure the unit and API tests pass.
   
   ## If this is a bug fix, what versions of Traffic Control are affected?
   <!-- If this PR fixes a bug, please list here all of the affected versions - to
   the best of your knowledge. It's also pretty helpful to include a commit hash
   of where 'master' is at the time this PR is opened (if it affects master),
   because what 'master' means will change over time. For example, if this PR
   fixes a bug that's present in master (at commit hash '1df853c8'), in v4.0.0,
   and in the current 4.0.1 Release candidate (e.g. RC1), then this list would
   look like:
   
   - master (1df853c8)
   - 4.0.0
   - 4.0.1 (RC1)
   
   If you don't know what other versions might have this bug, AND don't know how
   to find the commit hash of 'master', then feel free to leave this section
   blank (or, preferably, delete it entirely).
    -->
   
   - master
   ## The following criteria are ALL met by this PR
   <!-- Check the boxes to signify that the associated statement is true. To
   "check a box", replace the space inside of the square brackets with an 'x'.
   e.g.
   
   - [ x] <- Wrong
   - [x ] <- Wrong
   - [] <- Wrong
   - [*] <- Wrong
   - [x] <- Correct!
   
   -->
   
   - [x] This PR does not include tests because the available tests already cover the functionality.
   - [x] Documentation already done in the initial PRs by Rawlin.
   - [x] This PR does not include an update to CHANGELOG.md.
   - [x] This PR includes any and all required license headers
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://www.apache.org/security/) for details)
   
   
   ## Additional Information
   <!-- If you would like to include any additional information on the PR for
   potential reviewers please put it here.
   
   Some examples of this would be:
   
   - Before and after screenshots/gifs of the Traffic Portal if it is affected
   - Links to other dependent Pull Requests
   - References to relevant context (e.g. new/updates to dependent libraries,
   mailing list records, blueprints)
   
   Feel free to leave this section blank (or, preferably, delete it entirely).
   -->
   
   <!--
   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.
   -->
   


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

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



[GitHub] [trafficcontrol] rawlinp merged pull request #5812: Implement SSLKeys methods for PostgreSQL Traffic Vault backend

Posted by GitBox <gi...@apache.org>.
rawlinp merged pull request #5812:
URL: https://github.com/apache/trafficcontrol/pull/5812


   


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

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



[GitHub] [trafficcontrol] srijeet0406 commented on pull request #5812: Implement SSLKeys methods for PostgreSQL Traffic Vault backend

Posted by GitBox <gi...@apache.org>.
srijeet0406 commented on pull request #5812:
URL: https://github.com/apache/trafficcontrol/pull/5812#issuecomment-837318731


   > Everything looks good, but after running the `TestDeliveryServices` test there is an sslkey still left in the TV DB:
   > 
   > ```
   > {"cdn": "sslkeytransfer", "key": "dsID", "version": 1, "authType": "Self Signed", "hostname": "*.test2.com",
   > ```
   > 
   > Looking at your test code, it looks like all the certs should be getting cleaned up, but there are a couple certs created using `"dsID"` as the key. So I'm not sure which one isn't getting cleaned up.
   
   Yeah good catch. I've changed the cleanup method to remove this now ^


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

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



[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5812: Implement SSLKeys methods for PostgreSQL Traffic Vault backend

Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5812:
URL: https://github.com/apache/trafficcontrol/pull/5812#discussion_r627795905



##########
File path: traffic_ops/testing/api/v4/deliveryservices_test.go
##########
@@ -171,13 +173,278 @@ func cleanUp(t *testing.T, ds tc.DeliveryServiceV4, oldCDNID int, newCDNID int)
 	if err != nil {
 		t.Error(err)
 	}
-	_, _, err = TOSession.DeleteCDN(oldCDNID)
+	if oldCDNID != -1 {
+		_, _, err = TOSession.DeleteCDN(oldCDNID)
+		if err != nil {
+			t.Error(err)
+		}
+	}
+	if newCDNID != -1 {
+		_, _, err = TOSession.DeleteCDN(newCDNID)
+		if err != nil {
+			t.Error(err)
+		}
+	}
+}
+
+func DeleteCDNOldSSLKeys(t *testing.T) {
+	cdn := createBlankCDN("sslkeytransfer", t)
+
+	types, _, err := TOSession.GetTypeByName("HTTP", nil)
 	if err != nil {
-		t.Error(err)
+		t.Fatal("unable to get types: " + err.Error())
+	}
+	if len(types) < 1 {
+		t.Fatal("expected at least one type")
 	}
-	_, _, err = TOSession.DeleteCDN(newCDNID)
+
+	customDS := tc.DeliveryServiceV4{}
+	customDS.Active = util.BoolPtr(true)
+	customDS.CDNID = util.IntPtr(cdn.ID)
+	customDS.DSCP = util.IntPtr(0)
+	customDS.DisplayName = util.StrPtr("displayName")
+	customDS.RoutingName = util.StrPtr("routingName")
+	customDS.GeoLimit = util.IntPtr(0)
+	customDS.GeoProvider = util.IntPtr(0)
+	customDS.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS.InitialDispersion = util.IntPtr(1)
+	customDS.LogsEnabled = util.BoolPtr(true)
+	customDS.MissLat = util.FloatPtr(0)
+	customDS.MissLong = util.FloatPtr(0)
+	customDS.MultiSiteOrigin = util.BoolPtr(false)
+	customDS.OrgServerFQDN = util.StrPtr("https://test.com")
+	customDS.Protocol = util.IntPtr(2)
+	customDS.QStringIgnore = util.IntPtr(0)
+	customDS.RangeRequestHandling = util.IntPtr(0)
+	customDS.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS.TenantID = util.IntPtr(1)
+	customDS.TypeID = util.IntPtr(types[0].ID)
+	customDS.XMLID = util.StrPtr("dsID")
+	customDS.MaxRequestHeaderBytes = nil
+
+	ds, _, err := TOSession.CreateDeliveryService(customDS)
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
+	}
+	ds.CDNName = &cdn.Name
+	_, _, err = TOSession.GenerateSSLKeysForDS(*ds.XMLID, *ds.CDNName, tc.SSLKeyRequestFields{
+		BusinessUnit: util.StrPtr("BU"),
+		City:         util.StrPtr("CI"),
+		Organization: util.StrPtr("OR"),
+		HostName:     util.StrPtr("*.test.com"),
+		Country:      util.StrPtr("CO"),
+		State:        util.StrPtr("ST"),
+	})
+	if err != nil {
+		t.Fatalf("unable to generate sslkeys for DS %v: %v", *customDS.XMLID, err)
+	}
+	defer cleanUp(t, ds, cdn.ID, -1)
+
+	// Second DS creation
+	customDS2 := tc.DeliveryServiceV4{}
+	customDS2.Active = util.BoolPtr(true)
+	customDS2.CDNID = util.IntPtr(cdn.ID)
+	customDS2.DSCP = util.IntPtr(0)
+	customDS2.DisplayName = util.StrPtr("displayName2")
+	customDS2.RoutingName = util.StrPtr("routingName2")
+	customDS2.GeoLimit = util.IntPtr(0)
+	customDS2.GeoProvider = util.IntPtr(0)
+	customDS2.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS2.InitialDispersion = util.IntPtr(1)
+	customDS2.LogsEnabled = util.BoolPtr(true)
+	customDS2.MissLat = util.FloatPtr(0)
+	customDS2.MissLong = util.FloatPtr(0)
+	customDS2.MultiSiteOrigin = util.BoolPtr(false)
+	customDS2.OrgServerFQDN = util.StrPtr("https://test2.com")
+	customDS2.Protocol = util.IntPtr(2)
+	customDS2.QStringIgnore = util.IntPtr(0)
+	customDS2.RangeRequestHandling = util.IntPtr(0)
+	customDS2.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS2.TenantID = util.IntPtr(1)
+	customDS2.TypeID = util.IntPtr(types[0].ID)
+	customDS2.XMLID = util.StrPtr("dsID2")
+	customDS2.MaxRequestHeaderBytes = nil
+
+	ds2, _, err := TOSession.CreateDeliveryService(customDS2)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ds2.CDNName = &cdn.Name
+	_, _, err = TOSession.GenerateSSLKeysForDS(*ds2.XMLID, *ds2.CDNName, tc.SSLKeyRequestFields{
+		BusinessUnit: util.StrPtr("BU"),
+		City:         util.StrPtr("CI"),
+		Organization: util.StrPtr("OR"),
+		HostName:     util.StrPtr("*.test2.com"),
+		Country:      util.StrPtr("CO"),
+		State:        util.StrPtr("ST"),
+	})
+	if err != nil {
+		t.Fatalf("unable to generate sslkeys for DS %v: %v", *customDS2.XMLID, err)
+	}
+
+	tries := 0
+	var cdnKeys []tc.CDNSSLKeys
+	for tries < 5 {

Review comment:
       This loop could be more traditional like `for tries := 0; tries < 5; tries++`

##########
File path: traffic_ops/v4-client/deliveryservice.go
##########
@@ -289,6 +292,18 @@ func (to *Session) GenerateSSLKeysForDS(XMLID string, CDNName string, sslFields
 	return response.Response, reqInf, nil
 }
 
+// AddSSLKeysForDS adds SSL Keys for the given DS
+func (to *Session) AddSSLKeysForDS(request tc.DeliveryServiceAddSSLKeysReq) (string, toclientlib.ReqInf, error) {

Review comment:
       I think @ocket8888 may have to come in and clean this up once this is merged, but it might help him if this returned something like
   ```
   struct {
       Response string `json:"response"`
       Alerts
   }
   ```
   instead of just a string. That way, alerts (if any) are actually unmarshalled and checkable. I think there is already a type like that that could be aliased if need be.

##########
File path: traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go
##########
@@ -100,24 +101,149 @@ func (p *Postgres) commitTransaction(tx *sqlx.Tx, ctx context.Context, cancelFun
 	cancelFunc()
 }
 
+// GetDeliveryServiceSSLKeys retrieves the SSL keys of the given version for
+// the delivery service identified by the given xmlID. If version is empty,
+// the implementation should return the latest version.
 func (p *Postgres) GetDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) (tc.DeliveryServiceSSLKeysV15, bool, error) {
-	return tc.DeliveryServiceSSLKeysV15{}, false, notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	var jsonKeys string
+	query := "SELECT data FROM sslkey WHERE deliveryservice=$1"
+	if version != "" {
+		query += " AND version=$2"
+		err = tvTx.QueryRow(query, xmlID, version).Scan(&jsonKeys)
+	} else {
+		err = tvTx.QueryRow(query, xmlID).Scan(&jsonKeys)
+	}
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return tc.DeliveryServiceSSLKeysV15{}, false, nil
+		}
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing SELECT SSL Keys query", err, ctx.Err())
+		return tc.DeliveryServiceSSLKeysV15{}, false, e
+	}
+	sslKey := tc.DeliveryServiceSSLKeysV15{}
+	err = json.Unmarshal([]byte(jsonKeys), &sslKey)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, errors.New("unmarshalling ssl keys: " + err.Error())
+	}
+	return sslKey, true, nil
 }
 
+// PutDeliveryServiceSSLKeys stores the given SSL keys for a delivery service.
 func (p *Postgres) PutDeliveryServiceSSLKeys(key tc.DeliveryServiceSSLKeys, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+
+	keyJSON, err := json.Marshal(&key)

Review comment:
       I think callers of this method are still free to leave `version` empty, in which case we would write it to the DB w/ `version = "latest"`.

##########
File path: traffic_ops/testing/api/v4/deliveryservices_test.go
##########
@@ -171,13 +173,278 @@ func cleanUp(t *testing.T, ds tc.DeliveryServiceV4, oldCDNID int, newCDNID int)
 	if err != nil {
 		t.Error(err)
 	}
-	_, _, err = TOSession.DeleteCDN(oldCDNID)
+	if oldCDNID != -1 {
+		_, _, err = TOSession.DeleteCDN(oldCDNID)
+		if err != nil {
+			t.Error(err)
+		}
+	}
+	if newCDNID != -1 {
+		_, _, err = TOSession.DeleteCDN(newCDNID)
+		if err != nil {
+			t.Error(err)
+		}
+	}
+}
+
+func DeleteCDNOldSSLKeys(t *testing.T) {
+	cdn := createBlankCDN("sslkeytransfer", t)
+
+	types, _, err := TOSession.GetTypeByName("HTTP", nil)
 	if err != nil {
-		t.Error(err)
+		t.Fatal("unable to get types: " + err.Error())
+	}
+	if len(types) < 1 {
+		t.Fatal("expected at least one type")
 	}
-	_, _, err = TOSession.DeleteCDN(newCDNID)
+
+	customDS := tc.DeliveryServiceV4{}
+	customDS.Active = util.BoolPtr(true)
+	customDS.CDNID = util.IntPtr(cdn.ID)
+	customDS.DSCP = util.IntPtr(0)
+	customDS.DisplayName = util.StrPtr("displayName")
+	customDS.RoutingName = util.StrPtr("routingName")
+	customDS.GeoLimit = util.IntPtr(0)
+	customDS.GeoProvider = util.IntPtr(0)
+	customDS.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS.InitialDispersion = util.IntPtr(1)
+	customDS.LogsEnabled = util.BoolPtr(true)
+	customDS.MissLat = util.FloatPtr(0)
+	customDS.MissLong = util.FloatPtr(0)
+	customDS.MultiSiteOrigin = util.BoolPtr(false)
+	customDS.OrgServerFQDN = util.StrPtr("https://test.com")
+	customDS.Protocol = util.IntPtr(2)
+	customDS.QStringIgnore = util.IntPtr(0)
+	customDS.RangeRequestHandling = util.IntPtr(0)
+	customDS.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS.TenantID = util.IntPtr(1)
+	customDS.TypeID = util.IntPtr(types[0].ID)
+	customDS.XMLID = util.StrPtr("dsID")
+	customDS.MaxRequestHeaderBytes = nil
+
+	ds, _, err := TOSession.CreateDeliveryService(customDS)
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
+	}
+	ds.CDNName = &cdn.Name
+	_, _, err = TOSession.GenerateSSLKeysForDS(*ds.XMLID, *ds.CDNName, tc.SSLKeyRequestFields{
+		BusinessUnit: util.StrPtr("BU"),
+		City:         util.StrPtr("CI"),
+		Organization: util.StrPtr("OR"),
+		HostName:     util.StrPtr("*.test.com"),
+		Country:      util.StrPtr("CO"),
+		State:        util.StrPtr("ST"),
+	})
+	if err != nil {
+		t.Fatalf("unable to generate sslkeys for DS %v: %v", *customDS.XMLID, err)
+	}
+	defer cleanUp(t, ds, cdn.ID, -1)
+
+	// Second DS creation
+	customDS2 := tc.DeliveryServiceV4{}
+	customDS2.Active = util.BoolPtr(true)
+	customDS2.CDNID = util.IntPtr(cdn.ID)
+	customDS2.DSCP = util.IntPtr(0)
+	customDS2.DisplayName = util.StrPtr("displayName2")
+	customDS2.RoutingName = util.StrPtr("routingName2")
+	customDS2.GeoLimit = util.IntPtr(0)
+	customDS2.GeoProvider = util.IntPtr(0)
+	customDS2.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS2.InitialDispersion = util.IntPtr(1)
+	customDS2.LogsEnabled = util.BoolPtr(true)
+	customDS2.MissLat = util.FloatPtr(0)
+	customDS2.MissLong = util.FloatPtr(0)
+	customDS2.MultiSiteOrigin = util.BoolPtr(false)
+	customDS2.OrgServerFQDN = util.StrPtr("https://test2.com")
+	customDS2.Protocol = util.IntPtr(2)
+	customDS2.QStringIgnore = util.IntPtr(0)
+	customDS2.RangeRequestHandling = util.IntPtr(0)
+	customDS2.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS2.TenantID = util.IntPtr(1)
+	customDS2.TypeID = util.IntPtr(types[0].ID)
+	customDS2.XMLID = util.StrPtr("dsID2")
+	customDS2.MaxRequestHeaderBytes = nil
+
+	ds2, _, err := TOSession.CreateDeliveryService(customDS2)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ds2.CDNName = &cdn.Name
+	_, _, err = TOSession.GenerateSSLKeysForDS(*ds2.XMLID, *ds2.CDNName, tc.SSLKeyRequestFields{
+		BusinessUnit: util.StrPtr("BU"),
+		City:         util.StrPtr("CI"),
+		Organization: util.StrPtr("OR"),
+		HostName:     util.StrPtr("*.test2.com"),
+		Country:      util.StrPtr("CO"),
+		State:        util.StrPtr("ST"),
+	})
+	if err != nil {
+		t.Fatalf("unable to generate sslkeys for DS %v: %v", *customDS2.XMLID, err)
+	}
+
+	tries := 0
+	var cdnKeys []tc.CDNSSLKeys
+	for tries < 5 {
+		time.Sleep(time.Second)
+		cdnKeys, _, err = TOSession.GetCDNSSLKeys(cdn.Name, nil)
+		if err == nil && len(cdnKeys) != 0 {
+			break
+		}
+		tries++
+	}
+	if err != nil {
+		t.Fatalf("unable to get CDN %v SSL keys: %v", cdn.Name, err)
+	}
+	if len(cdnKeys) != 2 {
+		t.Errorf("expected two ssl keys for CDN %v, got %d instead", cdn.Name, len(cdnKeys))
+	}
+
+	_, err = TOSession.DeleteDeliveryService(*ds2.ID)
+	if err != nil {
+		t.Errorf("could not delete delivery service %v", *ds.XMLID)
+	}
+	_, _, err = TOSession.SnapshotCRConfigByID(cdn.ID)
+	if err != nil {
+		t.Fatalf("couldn't snap CDN: %v", err)
+	}
+	tries = 0
+	var newCdnKeys []tc.CDNSSLKeys
+	for tries < 5 {

Review comment:
       This loop could be more traditional like `for tries := 0; tries < 5; tries++`

##########
File path: traffic_ops/testing/api/v4/deliveryservices_test.go
##########
@@ -171,13 +173,278 @@ func cleanUp(t *testing.T, ds tc.DeliveryServiceV4, oldCDNID int, newCDNID int)
 	if err != nil {
 		t.Error(err)
 	}
-	_, _, err = TOSession.DeleteCDN(oldCDNID)
+	if oldCDNID != -1 {
+		_, _, err = TOSession.DeleteCDN(oldCDNID)
+		if err != nil {
+			t.Error(err)
+		}
+	}
+	if newCDNID != -1 {
+		_, _, err = TOSession.DeleteCDN(newCDNID)
+		if err != nil {
+			t.Error(err)
+		}
+	}
+}
+
+func DeleteCDNOldSSLKeys(t *testing.T) {
+	cdn := createBlankCDN("sslkeytransfer", t)
+
+	types, _, err := TOSession.GetTypeByName("HTTP", nil)
 	if err != nil {
-		t.Error(err)
+		t.Fatal("unable to get types: " + err.Error())
+	}
+	if len(types) < 1 {
+		t.Fatal("expected at least one type")
 	}
-	_, _, err = TOSession.DeleteCDN(newCDNID)
+
+	customDS := tc.DeliveryServiceV4{}
+	customDS.Active = util.BoolPtr(true)
+	customDS.CDNID = util.IntPtr(cdn.ID)
+	customDS.DSCP = util.IntPtr(0)
+	customDS.DisplayName = util.StrPtr("displayName")
+	customDS.RoutingName = util.StrPtr("routingName")
+	customDS.GeoLimit = util.IntPtr(0)
+	customDS.GeoProvider = util.IntPtr(0)
+	customDS.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS.InitialDispersion = util.IntPtr(1)
+	customDS.LogsEnabled = util.BoolPtr(true)
+	customDS.MissLat = util.FloatPtr(0)
+	customDS.MissLong = util.FloatPtr(0)
+	customDS.MultiSiteOrigin = util.BoolPtr(false)
+	customDS.OrgServerFQDN = util.StrPtr("https://test.com")
+	customDS.Protocol = util.IntPtr(2)
+	customDS.QStringIgnore = util.IntPtr(0)
+	customDS.RangeRequestHandling = util.IntPtr(0)
+	customDS.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS.TenantID = util.IntPtr(1)
+	customDS.TypeID = util.IntPtr(types[0].ID)
+	customDS.XMLID = util.StrPtr("dsID")
+	customDS.MaxRequestHeaderBytes = nil
+
+	ds, _, err := TOSession.CreateDeliveryService(customDS)
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
+	}
+	ds.CDNName = &cdn.Name
+	_, _, err = TOSession.GenerateSSLKeysForDS(*ds.XMLID, *ds.CDNName, tc.SSLKeyRequestFields{
+		BusinessUnit: util.StrPtr("BU"),
+		City:         util.StrPtr("CI"),
+		Organization: util.StrPtr("OR"),
+		HostName:     util.StrPtr("*.test.com"),
+		Country:      util.StrPtr("CO"),
+		State:        util.StrPtr("ST"),
+	})
+	if err != nil {
+		t.Fatalf("unable to generate sslkeys for DS %v: %v", *customDS.XMLID, err)
+	}
+	defer cleanUp(t, ds, cdn.ID, -1)
+
+	// Second DS creation
+	customDS2 := tc.DeliveryServiceV4{}
+	customDS2.Active = util.BoolPtr(true)
+	customDS2.CDNID = util.IntPtr(cdn.ID)
+	customDS2.DSCP = util.IntPtr(0)
+	customDS2.DisplayName = util.StrPtr("displayName2")
+	customDS2.RoutingName = util.StrPtr("routingName2")
+	customDS2.GeoLimit = util.IntPtr(0)
+	customDS2.GeoProvider = util.IntPtr(0)
+	customDS2.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS2.InitialDispersion = util.IntPtr(1)
+	customDS2.LogsEnabled = util.BoolPtr(true)
+	customDS2.MissLat = util.FloatPtr(0)
+	customDS2.MissLong = util.FloatPtr(0)
+	customDS2.MultiSiteOrigin = util.BoolPtr(false)
+	customDS2.OrgServerFQDN = util.StrPtr("https://test2.com")
+	customDS2.Protocol = util.IntPtr(2)
+	customDS2.QStringIgnore = util.IntPtr(0)
+	customDS2.RangeRequestHandling = util.IntPtr(0)
+	customDS2.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS2.TenantID = util.IntPtr(1)
+	customDS2.TypeID = util.IntPtr(types[0].ID)
+	customDS2.XMLID = util.StrPtr("dsID2")
+	customDS2.MaxRequestHeaderBytes = nil

Review comment:
       +1 on this

##########
File path: traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go
##########
@@ -100,24 +101,149 @@ func (p *Postgres) commitTransaction(tx *sqlx.Tx, ctx context.Context, cancelFun
 	cancelFunc()
 }
 
+// GetDeliveryServiceSSLKeys retrieves the SSL keys of the given version for
+// the delivery service identified by the given xmlID. If version is empty,
+// the implementation should return the latest version.
 func (p *Postgres) GetDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) (tc.DeliveryServiceSSLKeysV15, bool, error) {
-	return tc.DeliveryServiceSSLKeysV15{}, false, notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	var jsonKeys string
+	query := "SELECT data FROM sslkey WHERE deliveryservice=$1"
+	if version != "" {
+		query += " AND version=$2"
+		err = tvTx.QueryRow(query, xmlID, version).Scan(&jsonKeys)
+	} else {
+		err = tvTx.QueryRow(query, xmlID).Scan(&jsonKeys)
+	}
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return tc.DeliveryServiceSSLKeysV15{}, false, nil
+		}
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing SELECT SSL Keys query", err, ctx.Err())
+		return tc.DeliveryServiceSSLKeysV15{}, false, e
+	}
+	sslKey := tc.DeliveryServiceSSLKeysV15{}
+	err = json.Unmarshal([]byte(jsonKeys), &sslKey)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, errors.New("unmarshalling ssl keys: " + err.Error())
+	}
+	return sslKey, true, nil
 }
 
+// PutDeliveryServiceSSLKeys stores the given SSL keys for a delivery service.
 func (p *Postgres) PutDeliveryServiceSSLKeys(key tc.DeliveryServiceSSLKeys, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+
+	keyJSON, err := json.Marshal(&key)
+	if err != nil {
+		return errors.New("marshalling keys: " + err.Error())
+	}
+
+	// delete the old ssl keys first
+	_, err = tvTx.Exec("DELETE FROM sslkey WHERE deliveryservice=$1", key.DeliveryService)

Review comment:
       So I think we will need to delete two rows possibly. If the given version is `""`, it is assumed to be `"latest"`. So we will always need to delete the key w/ version `"latest"` first. If version is _not empty_, then we'll also have to delete the key (if any) at _that given version_.
   
   Then, we will `INSERT` the key with version `"latest"` _and_ at the given version (if non-empty).

##########
File path: traffic_ops/testing/api/v4/deliveryservices_test.go
##########
@@ -171,13 +173,278 @@ func cleanUp(t *testing.T, ds tc.DeliveryServiceV4, oldCDNID int, newCDNID int)
 	if err != nil {
 		t.Error(err)
 	}
-	_, _, err = TOSession.DeleteCDN(oldCDNID)
+	if oldCDNID != -1 {
+		_, _, err = TOSession.DeleteCDN(oldCDNID)
+		if err != nil {
+			t.Error(err)
+		}
+	}
+	if newCDNID != -1 {
+		_, _, err = TOSession.DeleteCDN(newCDNID)
+		if err != nil {
+			t.Error(err)
+		}
+	}
+}
+
+func DeleteCDNOldSSLKeys(t *testing.T) {
+	cdn := createBlankCDN("sslkeytransfer", t)
+
+	types, _, err := TOSession.GetTypeByName("HTTP", nil)
 	if err != nil {
-		t.Error(err)
+		t.Fatal("unable to get types: " + err.Error())
+	}
+	if len(types) < 1 {
+		t.Fatal("expected at least one type")
 	}
-	_, _, err = TOSession.DeleteCDN(newCDNID)
+
+	customDS := tc.DeliveryServiceV4{}
+	customDS.Active = util.BoolPtr(true)
+	customDS.CDNID = util.IntPtr(cdn.ID)
+	customDS.DSCP = util.IntPtr(0)
+	customDS.DisplayName = util.StrPtr("displayName")
+	customDS.RoutingName = util.StrPtr("routingName")
+	customDS.GeoLimit = util.IntPtr(0)
+	customDS.GeoProvider = util.IntPtr(0)
+	customDS.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS.InitialDispersion = util.IntPtr(1)
+	customDS.LogsEnabled = util.BoolPtr(true)
+	customDS.MissLat = util.FloatPtr(0)
+	customDS.MissLong = util.FloatPtr(0)
+	customDS.MultiSiteOrigin = util.BoolPtr(false)
+	customDS.OrgServerFQDN = util.StrPtr("https://test.com")
+	customDS.Protocol = util.IntPtr(2)
+	customDS.QStringIgnore = util.IntPtr(0)
+	customDS.RangeRequestHandling = util.IntPtr(0)
+	customDS.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS.TenantID = util.IntPtr(1)
+	customDS.TypeID = util.IntPtr(types[0].ID)
+	customDS.XMLID = util.StrPtr("dsID")
+	customDS.MaxRequestHeaderBytes = nil
+
+	ds, _, err := TOSession.CreateDeliveryService(customDS)
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
+	}
+	ds.CDNName = &cdn.Name
+	_, _, err = TOSession.GenerateSSLKeysForDS(*ds.XMLID, *ds.CDNName, tc.SSLKeyRequestFields{
+		BusinessUnit: util.StrPtr("BU"),
+		City:         util.StrPtr("CI"),
+		Organization: util.StrPtr("OR"),
+		HostName:     util.StrPtr("*.test.com"),
+		Country:      util.StrPtr("CO"),
+		State:        util.StrPtr("ST"),
+	})
+	if err != nil {
+		t.Fatalf("unable to generate sslkeys for DS %v: %v", *customDS.XMLID, err)
+	}
+	defer cleanUp(t, ds, cdn.ID, -1)
+
+	// Second DS creation
+	customDS2 := tc.DeliveryServiceV4{}
+	customDS2.Active = util.BoolPtr(true)
+	customDS2.CDNID = util.IntPtr(cdn.ID)
+	customDS2.DSCP = util.IntPtr(0)
+	customDS2.DisplayName = util.StrPtr("displayName2")
+	customDS2.RoutingName = util.StrPtr("routingName2")
+	customDS2.GeoLimit = util.IntPtr(0)
+	customDS2.GeoProvider = util.IntPtr(0)
+	customDS2.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS2.InitialDispersion = util.IntPtr(1)
+	customDS2.LogsEnabled = util.BoolPtr(true)
+	customDS2.MissLat = util.FloatPtr(0)
+	customDS2.MissLong = util.FloatPtr(0)
+	customDS2.MultiSiteOrigin = util.BoolPtr(false)
+	customDS2.OrgServerFQDN = util.StrPtr("https://test2.com")
+	customDS2.Protocol = util.IntPtr(2)
+	customDS2.QStringIgnore = util.IntPtr(0)
+	customDS2.RangeRequestHandling = util.IntPtr(0)
+	customDS2.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS2.TenantID = util.IntPtr(1)
+	customDS2.TypeID = util.IntPtr(types[0].ID)
+	customDS2.XMLID = util.StrPtr("dsID2")
+	customDS2.MaxRequestHeaderBytes = nil
+
+	ds2, _, err := TOSession.CreateDeliveryService(customDS2)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ds2.CDNName = &cdn.Name
+	_, _, err = TOSession.GenerateSSLKeysForDS(*ds2.XMLID, *ds2.CDNName, tc.SSLKeyRequestFields{
+		BusinessUnit: util.StrPtr("BU"),
+		City:         util.StrPtr("CI"),
+		Organization: util.StrPtr("OR"),
+		HostName:     util.StrPtr("*.test2.com"),
+		Country:      util.StrPtr("CO"),
+		State:        util.StrPtr("ST"),
+	})
+	if err != nil {
+		t.Fatalf("unable to generate sslkeys for DS %v: %v", *customDS2.XMLID, err)
+	}
+
+	tries := 0
+	var cdnKeys []tc.CDNSSLKeys
+	for tries < 5 {
+		time.Sleep(time.Second)
+		cdnKeys, _, err = TOSession.GetCDNSSLKeys(cdn.Name, nil)
+		if err == nil && len(cdnKeys) != 0 {
+			break
+		}
+		tries++
+	}
+	if err != nil {
+		t.Fatalf("unable to get CDN %v SSL keys: %v", cdn.Name, err)
+	}
+	if len(cdnKeys) != 2 {
+		t.Errorf("expected two ssl keys for CDN %v, got %d instead", cdn.Name, len(cdnKeys))
+	}
+
+	_, err = TOSession.DeleteDeliveryService(*ds2.ID)
+	if err != nil {
+		t.Errorf("could not delete delivery service %v", *ds.XMLID)
+	}
+	_, _, err = TOSession.SnapshotCRConfigByID(cdn.ID)
+	if err != nil {
+		t.Fatalf("couldn't snap CDN: %v", err)
+	}
+	tries = 0
+	var newCdnKeys []tc.CDNSSLKeys
+	for tries < 5 {
+		time.Sleep(time.Second)
+		newCdnKeys, _, err = TOSession.GetCDNSSLKeys(cdn.Name, nil)
+		tries++
+	}
+	if err != nil {
+		t.Fatalf("unable to get CDN %v SSL keys: %v", cdn.Name, err)
+	}
+	if len(newCdnKeys) != 1 {
+		t.Errorf("expected 1 ssl keys for CDN %v, got %d instead", cdn.Name, len(newCdnKeys))
+	}
+}
+
+func DeliveryServiceSSLKeys(t *testing.T) {
+	cdn := createBlankCDN("sslkeytransfer2", t)
+
+	types, _, err := TOSession.GetTypeByName("HTTP", nil)
+	if err != nil {
+		t.Fatal("unable to get types: " + err.Error())
+	}
+	if len(types) < 1 {
+		t.Fatal("expected at least one type")
+	}
+
+	customDS := tc.DeliveryServiceV4{}
+	customDS.Active = util.BoolPtr(true)
+	customDS.CDNID = util.IntPtr(cdn.ID)
+	customDS.DSCP = util.IntPtr(0)
+	customDS.DisplayName = util.StrPtr("displayName2")
+	customDS.RoutingName = util.StrPtr("routingName2")
+	customDS.GeoLimit = util.IntPtr(0)
+	customDS.GeoProvider = util.IntPtr(0)
+	customDS.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS.InitialDispersion = util.IntPtr(1)
+	customDS.LogsEnabled = util.BoolPtr(true)
+	customDS.MissLat = util.FloatPtr(0)
+	customDS.MissLong = util.FloatPtr(0)
+	customDS.MultiSiteOrigin = util.BoolPtr(false)
+	customDS.OrgServerFQDN = util.StrPtr("https://test2.com")
+	customDS.Protocol = util.IntPtr(2)
+	customDS.QStringIgnore = util.IntPtr(0)
+	customDS.RangeRequestHandling = util.IntPtr(0)
+	customDS.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS.TenantID = util.IntPtr(1)
+	customDS.TypeID = util.IntPtr(types[0].ID)
+	customDS.XMLID = util.StrPtr("dsID2")
+	customDS.MaxRequestHeaderBytes = nil
+
+	ds, _, err := TOSession.CreateDeliveryService(customDS)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ds.CDNName = &cdn.Name
+	_, _, err = TOSession.GenerateSSLKeysForDS(*ds.XMLID, *ds.CDNName, tc.SSLKeyRequestFields{
+		BusinessUnit: util.StrPtr("BU"),
+		City:         util.StrPtr("CI"),
+		Organization: util.StrPtr("OR"),
+		HostName:     util.StrPtr("*.test2.com"),
+		Country:      util.StrPtr("CO"),
+		State:        util.StrPtr("ST"),
+	})
+	if err != nil {
+		t.Fatalf("unable to generate sslkeys for DS %v: %v", *customDS.XMLID, err)
+	}
+	defer cleanUp(t, ds, cdn.ID, -1)
+
+	if ds.XMLID == nil {
+		t.Fatalf("got a DS with an invalid xml ID")
+	}
+
+	tries := 0
+	var dsSSLKey *tc.DeliveryServiceSSLKeys
+	for tries < 5 {
+		time.Sleep(time.Second)
+		dsSSLKey, _, err = TOSession.GetDeliveryServiceSSLKeys(*ds.XMLID, nil)
+		if err == nil && dsSSLKey != nil {
+			break
+		}
+		tries++
+	}
+
+	if err != nil || dsSSLKey == nil {
+		t.Fatalf("unable to get DS %v SSL key: %v", *ds.XMLID, err)
+	}
+	if dsSSLKey.Certificate.Key == "" {
+		t.Errorf("expected a valid key but got nothing")
+	}
+	if dsSSLKey.Certificate.Crt == "" {
+		t.Errorf("expected a valid certificate, but got nothing")
+	}
+	if dsSSLKey.Certificate.CSR == "" {
+		t.Errorf("expected a valid CSR, but got nothing")
+	}
+
+	err = deliveryservice.Base64DecodeCertificate(&dsSSLKey.Certificate)
+	if err != nil {
+		t.Fatalf("couldn't decode certificate: %v", err)
+	}
+
+	dsSSLKeyReq := tc.DeliveryServiceSSLKeysReq{
+		AuthType:        &dsSSLKey.AuthType,
+		CDN:             &dsSSLKey.CDN,
+		DeliveryService: &dsSSLKey.DeliveryService,
+		BusinessUnit:    &dsSSLKey.BusinessUnit,
+		City:            &dsSSLKey.City,
+		Organization:    &dsSSLKey.Organization,
+		HostName:        &dsSSLKey.Hostname,
+		Country:         &dsSSLKey.Country,
+		State:           &dsSSLKey.State,
+		Key:             &dsSSLKey.Key,
+		Version:         &dsSSLKey.Version,
+		Certificate:     &dsSSLKey.Certificate,
+	}
+	_, _, err = TOSession.AddSSLKeysForDS(tc.DeliveryServiceAddSSLKeysReq{DeliveryServiceSSLKeysReq: dsSSLKeyReq})
+
+	tries = 0
+	for tries < 5 {

Review comment:
       This loop could be more traditional like `for tries := 0; tries < 5; tries++`

##########
File path: traffic_ops/testing/api/v4/deliveryservices_test.go
##########
@@ -171,13 +173,278 @@ func cleanUp(t *testing.T, ds tc.DeliveryServiceV4, oldCDNID int, newCDNID int)
 	if err != nil {
 		t.Error(err)
 	}
-	_, _, err = TOSession.DeleteCDN(oldCDNID)
+	if oldCDNID != -1 {
+		_, _, err = TOSession.DeleteCDN(oldCDNID)
+		if err != nil {
+			t.Error(err)
+		}
+	}
+	if newCDNID != -1 {
+		_, _, err = TOSession.DeleteCDN(newCDNID)
+		if err != nil {
+			t.Error(err)
+		}
+	}
+}
+
+func DeleteCDNOldSSLKeys(t *testing.T) {
+	cdn := createBlankCDN("sslkeytransfer", t)
+
+	types, _, err := TOSession.GetTypeByName("HTTP", nil)
 	if err != nil {
-		t.Error(err)
+		t.Fatal("unable to get types: " + err.Error())
+	}
+	if len(types) < 1 {
+		t.Fatal("expected at least one type")
 	}
-	_, _, err = TOSession.DeleteCDN(newCDNID)
+
+	customDS := tc.DeliveryServiceV4{}
+	customDS.Active = util.BoolPtr(true)
+	customDS.CDNID = util.IntPtr(cdn.ID)
+	customDS.DSCP = util.IntPtr(0)
+	customDS.DisplayName = util.StrPtr("displayName")
+	customDS.RoutingName = util.StrPtr("routingName")
+	customDS.GeoLimit = util.IntPtr(0)
+	customDS.GeoProvider = util.IntPtr(0)
+	customDS.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS.InitialDispersion = util.IntPtr(1)
+	customDS.LogsEnabled = util.BoolPtr(true)
+	customDS.MissLat = util.FloatPtr(0)
+	customDS.MissLong = util.FloatPtr(0)
+	customDS.MultiSiteOrigin = util.BoolPtr(false)
+	customDS.OrgServerFQDN = util.StrPtr("https://test.com")
+	customDS.Protocol = util.IntPtr(2)
+	customDS.QStringIgnore = util.IntPtr(0)
+	customDS.RangeRequestHandling = util.IntPtr(0)
+	customDS.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS.TenantID = util.IntPtr(1)
+	customDS.TypeID = util.IntPtr(types[0].ID)
+	customDS.XMLID = util.StrPtr("dsID")
+	customDS.MaxRequestHeaderBytes = nil
+
+	ds, _, err := TOSession.CreateDeliveryService(customDS)
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
+	}
+	ds.CDNName = &cdn.Name
+	_, _, err = TOSession.GenerateSSLKeysForDS(*ds.XMLID, *ds.CDNName, tc.SSLKeyRequestFields{
+		BusinessUnit: util.StrPtr("BU"),
+		City:         util.StrPtr("CI"),
+		Organization: util.StrPtr("OR"),
+		HostName:     util.StrPtr("*.test.com"),
+		Country:      util.StrPtr("CO"),
+		State:        util.StrPtr("ST"),
+	})
+	if err != nil {
+		t.Fatalf("unable to generate sslkeys for DS %v: %v", *customDS.XMLID, err)
+	}
+	defer cleanUp(t, ds, cdn.ID, -1)
+
+	// Second DS creation
+	customDS2 := tc.DeliveryServiceV4{}
+	customDS2.Active = util.BoolPtr(true)
+	customDS2.CDNID = util.IntPtr(cdn.ID)
+	customDS2.DSCP = util.IntPtr(0)
+	customDS2.DisplayName = util.StrPtr("displayName2")
+	customDS2.RoutingName = util.StrPtr("routingName2")
+	customDS2.GeoLimit = util.IntPtr(0)
+	customDS2.GeoProvider = util.IntPtr(0)
+	customDS2.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS2.InitialDispersion = util.IntPtr(1)
+	customDS2.LogsEnabled = util.BoolPtr(true)
+	customDS2.MissLat = util.FloatPtr(0)
+	customDS2.MissLong = util.FloatPtr(0)
+	customDS2.MultiSiteOrigin = util.BoolPtr(false)
+	customDS2.OrgServerFQDN = util.StrPtr("https://test2.com")
+	customDS2.Protocol = util.IntPtr(2)
+	customDS2.QStringIgnore = util.IntPtr(0)
+	customDS2.RangeRequestHandling = util.IntPtr(0)
+	customDS2.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS2.TenantID = util.IntPtr(1)
+	customDS2.TypeID = util.IntPtr(types[0].ID)
+	customDS2.XMLID = util.StrPtr("dsID2")
+	customDS2.MaxRequestHeaderBytes = nil
+
+	ds2, _, err := TOSession.CreateDeliveryService(customDS2)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ds2.CDNName = &cdn.Name
+	_, _, err = TOSession.GenerateSSLKeysForDS(*ds2.XMLID, *ds2.CDNName, tc.SSLKeyRequestFields{
+		BusinessUnit: util.StrPtr("BU"),
+		City:         util.StrPtr("CI"),
+		Organization: util.StrPtr("OR"),
+		HostName:     util.StrPtr("*.test2.com"),
+		Country:      util.StrPtr("CO"),
+		State:        util.StrPtr("ST"),
+	})
+	if err != nil {
+		t.Fatalf("unable to generate sslkeys for DS %v: %v", *customDS2.XMLID, err)
+	}
+
+	tries := 0
+	var cdnKeys []tc.CDNSSLKeys
+	for tries < 5 {
+		time.Sleep(time.Second)
+		cdnKeys, _, err = TOSession.GetCDNSSLKeys(cdn.Name, nil)
+		if err == nil && len(cdnKeys) != 0 {
+			break
+		}
+		tries++
+	}
+	if err != nil {
+		t.Fatalf("unable to get CDN %v SSL keys: %v", cdn.Name, err)
+	}
+	if len(cdnKeys) != 2 {
+		t.Errorf("expected two ssl keys for CDN %v, got %d instead", cdn.Name, len(cdnKeys))
+	}
+
+	_, err = TOSession.DeleteDeliveryService(*ds2.ID)
+	if err != nil {
+		t.Errorf("could not delete delivery service %v", *ds.XMLID)
+	}
+	_, _, err = TOSession.SnapshotCRConfigByID(cdn.ID)
+	if err != nil {
+		t.Fatalf("couldn't snap CDN: %v", err)
+	}
+	tries = 0
+	var newCdnKeys []tc.CDNSSLKeys
+	for tries < 5 {
+		time.Sleep(time.Second)
+		newCdnKeys, _, err = TOSession.GetCDNSSLKeys(cdn.Name, nil)
+		tries++
+	}
+	if err != nil {
+		t.Fatalf("unable to get CDN %v SSL keys: %v", cdn.Name, err)
+	}
+	if len(newCdnKeys) != 1 {
+		t.Errorf("expected 1 ssl keys for CDN %v, got %d instead", cdn.Name, len(newCdnKeys))
+	}
+}
+
+func DeliveryServiceSSLKeys(t *testing.T) {
+	cdn := createBlankCDN("sslkeytransfer2", t)
+
+	types, _, err := TOSession.GetTypeByName("HTTP", nil)
+	if err != nil {
+		t.Fatal("unable to get types: " + err.Error())
+	}
+	if len(types) < 1 {
+		t.Fatal("expected at least one type")
+	}
+
+	customDS := tc.DeliveryServiceV4{}
+	customDS.Active = util.BoolPtr(true)
+	customDS.CDNID = util.IntPtr(cdn.ID)
+	customDS.DSCP = util.IntPtr(0)
+	customDS.DisplayName = util.StrPtr("displayName2")
+	customDS.RoutingName = util.StrPtr("routingName2")
+	customDS.GeoLimit = util.IntPtr(0)
+	customDS.GeoProvider = util.IntPtr(0)
+	customDS.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS.InitialDispersion = util.IntPtr(1)
+	customDS.LogsEnabled = util.BoolPtr(true)
+	customDS.MissLat = util.FloatPtr(0)
+	customDS.MissLong = util.FloatPtr(0)
+	customDS.MultiSiteOrigin = util.BoolPtr(false)
+	customDS.OrgServerFQDN = util.StrPtr("https://test2.com")
+	customDS.Protocol = util.IntPtr(2)
+	customDS.QStringIgnore = util.IntPtr(0)
+	customDS.RangeRequestHandling = util.IntPtr(0)
+	customDS.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS.TenantID = util.IntPtr(1)
+	customDS.TypeID = util.IntPtr(types[0].ID)
+	customDS.XMLID = util.StrPtr("dsID2")
+	customDS.MaxRequestHeaderBytes = nil
+
+	ds, _, err := TOSession.CreateDeliveryService(customDS)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ds.CDNName = &cdn.Name
+	_, _, err = TOSession.GenerateSSLKeysForDS(*ds.XMLID, *ds.CDNName, tc.SSLKeyRequestFields{
+		BusinessUnit: util.StrPtr("BU"),
+		City:         util.StrPtr("CI"),
+		Organization: util.StrPtr("OR"),
+		HostName:     util.StrPtr("*.test2.com"),
+		Country:      util.StrPtr("CO"),
+		State:        util.StrPtr("ST"),
+	})
+	if err != nil {
+		t.Fatalf("unable to generate sslkeys for DS %v: %v", *customDS.XMLID, err)
+	}
+	defer cleanUp(t, ds, cdn.ID, -1)
+
+	if ds.XMLID == nil {
+		t.Fatalf("got a DS with an invalid xml ID")
+	}
+
+	tries := 0
+	var dsSSLKey *tc.DeliveryServiceSSLKeys
+	for tries < 5 {

Review comment:
       This loop could be more traditional like `for tries := 0; tries < 5; tries++`

##########
File path: traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go
##########
@@ -100,24 +101,149 @@ func (p *Postgres) commitTransaction(tx *sqlx.Tx, ctx context.Context, cancelFun
 	cancelFunc()
 }
 
+// GetDeliveryServiceSSLKeys retrieves the SSL keys of the given version for
+// the delivery service identified by the given xmlID. If version is empty,
+// the implementation should return the latest version.
 func (p *Postgres) GetDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) (tc.DeliveryServiceSSLKeysV15, bool, error) {
-	return tc.DeliveryServiceSSLKeysV15{}, false, notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	var jsonKeys string
+	query := "SELECT data FROM sslkey WHERE deliveryservice=$1"
+	if version != "" {
+		query += " AND version=$2"
+		err = tvTx.QueryRow(query, xmlID, version).Scan(&jsonKeys)
+	} else {
+		err = tvTx.QueryRow(query, xmlID).Scan(&jsonKeys)
+	}
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return tc.DeliveryServiceSSLKeysV15{}, false, nil
+		}
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing SELECT SSL Keys query", err, ctx.Err())
+		return tc.DeliveryServiceSSLKeysV15{}, false, e
+	}
+	sslKey := tc.DeliveryServiceSSLKeysV15{}
+	err = json.Unmarshal([]byte(jsonKeys), &sslKey)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, errors.New("unmarshalling ssl keys: " + err.Error())
+	}
+	return sslKey, true, nil
 }
 
+// PutDeliveryServiceSSLKeys stores the given SSL keys for a delivery service.
 func (p *Postgres) PutDeliveryServiceSSLKeys(key tc.DeliveryServiceSSLKeys, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+
+	keyJSON, err := json.Marshal(&key)
+	if err != nil {
+		return errors.New("marshalling keys: " + err.Error())
+	}
+
+	// delete the old ssl keys first
+	_, err = tvTx.Exec("DELETE FROM sslkey WHERE deliveryservice=$1", key.DeliveryService)
+	if err != nil && err != sql.ErrNoRows {
+		return err
+	}
+	res, err := tvTx.Exec("INSERT INTO sslkey (deliveryservice, data, cdn, version) VALUES ($1, $2, $3, $4)", key.DeliveryService, keyJSON, key.CDN, key.Version)
+	if err != nil {
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing INSERT SSL Key query", err, ctx.Err())
+		return e
+	}
+	if rowsAffected, err := res.RowsAffected(); err != nil {
+		return err
+	} else if rowsAffected == 0 {
+		return errors.New("SSL Key: no keys were inserted")
+	}
+	return nil
 }
 
+// DeleteDeliveryServiceSSLKeys removes the SSL keys of the given version (or latest
+// if version is empty) for the delivery service identified by the given xmlID.
 func (p *Postgres) DeleteDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	query := "DELETE FROM sslkey WHERE deliveryservice=$1"
+	if version != "" {
+		query += " AND version=$2"
+		_, err = tvTx.Exec(query, xmlID, version)
+	} else {
+		_, err = tvTx.Exec(query, xmlID)
+	}
+	if err != nil {
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing DELETE SSL Key query", err, ctx.Err())
+		return e
+	}
+	return nil
 }
 
+// DeleteOldDeliveryServiceSSLKeys takes a set of existingXMLIDs as input and will remove
+// all SSL keys for delivery services in the CDN identified by the given cdnName that
+// do not contain an xmlID in the given set of existingXMLIDs. This method is called
+// during a snapshot operation in order to delete SSL keys for delivery services that
+// no longer exist.
 func (p *Postgres) DeleteOldDeliveryServiceSSLKeys(existingXMLIDs map[string]struct{}, cdnName string, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	var keys []string
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+
+	if len(existingXMLIDs) == 0 {
+		keys = append(keys, "")
+	}
+	for k, _ := range existingXMLIDs {
+		keys = append(keys, k)
+	}
+	_, err = tvTx.Exec("DELETE FROM sslkey WHERE cdn=$1 AND deliveryservice <> ALL ($2)", cdnName, pq.Array(keys))
+	if err != nil {
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing DELETE OLD SSL Key query", err, ctx.Err())
+		return e
+	}
+	return nil
 }
 
+// GetCDNSSLKeys retrieves all the SSL keys for delivery services in the CDN identified
+// by the given cdnName.
 func (p *Postgres) GetCDNSSLKeys(cdnName string, tx *sql.Tx, ctx context.Context) ([]tc.CDNSSLKey, error) {
-	return nil, notImplementedErr
+	var keys []tc.CDNSSLKey
+	var key tc.CDNSSLKey
+
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return keys, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+
+	rows, err := tvTx.Query("SELECT data from sslkey WHERE cdn=$1", cdnName)

Review comment:
       Yes, this query should only select keys `WHERE cdn=$1 AND version = 'latest'`

##########
File path: traffic_ops/testing/api/v4/deliveryservices_test.go
##########
@@ -171,13 +173,278 @@ func cleanUp(t *testing.T, ds tc.DeliveryServiceV4, oldCDNID int, newCDNID int)
 	if err != nil {
 		t.Error(err)
 	}
-	_, _, err = TOSession.DeleteCDN(oldCDNID)
+	if oldCDNID != -1 {
+		_, _, err = TOSession.DeleteCDN(oldCDNID)
+		if err != nil {
+			t.Error(err)
+		}
+	}
+	if newCDNID != -1 {
+		_, _, err = TOSession.DeleteCDN(newCDNID)
+		if err != nil {
+			t.Error(err)
+		}
+	}
+}
+
+func DeleteCDNOldSSLKeys(t *testing.T) {
+	cdn := createBlankCDN("sslkeytransfer", t)
+
+	types, _, err := TOSession.GetTypeByName("HTTP", nil)
 	if err != nil {
-		t.Error(err)
+		t.Fatal("unable to get types: " + err.Error())
+	}
+	if len(types) < 1 {
+		t.Fatal("expected at least one type")
 	}
-	_, _, err = TOSession.DeleteCDN(newCDNID)
+
+	customDS := tc.DeliveryServiceV4{}
+	customDS.Active = util.BoolPtr(true)
+	customDS.CDNID = util.IntPtr(cdn.ID)
+	customDS.DSCP = util.IntPtr(0)
+	customDS.DisplayName = util.StrPtr("displayName")
+	customDS.RoutingName = util.StrPtr("routingName")
+	customDS.GeoLimit = util.IntPtr(0)
+	customDS.GeoProvider = util.IntPtr(0)
+	customDS.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS.InitialDispersion = util.IntPtr(1)
+	customDS.LogsEnabled = util.BoolPtr(true)
+	customDS.MissLat = util.FloatPtr(0)
+	customDS.MissLong = util.FloatPtr(0)
+	customDS.MultiSiteOrigin = util.BoolPtr(false)
+	customDS.OrgServerFQDN = util.StrPtr("https://test.com")
+	customDS.Protocol = util.IntPtr(2)
+	customDS.QStringIgnore = util.IntPtr(0)
+	customDS.RangeRequestHandling = util.IntPtr(0)
+	customDS.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS.TenantID = util.IntPtr(1)
+	customDS.TypeID = util.IntPtr(types[0].ID)
+	customDS.XMLID = util.StrPtr("dsID")
+	customDS.MaxRequestHeaderBytes = nil
+
+	ds, _, err := TOSession.CreateDeliveryService(customDS)
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
+	}
+	ds.CDNName = &cdn.Name
+	_, _, err = TOSession.GenerateSSLKeysForDS(*ds.XMLID, *ds.CDNName, tc.SSLKeyRequestFields{
+		BusinessUnit: util.StrPtr("BU"),
+		City:         util.StrPtr("CI"),
+		Organization: util.StrPtr("OR"),
+		HostName:     util.StrPtr("*.test.com"),
+		Country:      util.StrPtr("CO"),
+		State:        util.StrPtr("ST"),
+	})
+	if err != nil {
+		t.Fatalf("unable to generate sslkeys for DS %v: %v", *customDS.XMLID, err)
+	}
+	defer cleanUp(t, ds, cdn.ID, -1)
+
+	// Second DS creation
+	customDS2 := tc.DeliveryServiceV4{}
+	customDS2.Active = util.BoolPtr(true)
+	customDS2.CDNID = util.IntPtr(cdn.ID)
+	customDS2.DSCP = util.IntPtr(0)
+	customDS2.DisplayName = util.StrPtr("displayName2")
+	customDS2.RoutingName = util.StrPtr("routingName2")
+	customDS2.GeoLimit = util.IntPtr(0)
+	customDS2.GeoProvider = util.IntPtr(0)
+	customDS2.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS2.InitialDispersion = util.IntPtr(1)
+	customDS2.LogsEnabled = util.BoolPtr(true)
+	customDS2.MissLat = util.FloatPtr(0)
+	customDS2.MissLong = util.FloatPtr(0)
+	customDS2.MultiSiteOrigin = util.BoolPtr(false)
+	customDS2.OrgServerFQDN = util.StrPtr("https://test2.com")
+	customDS2.Protocol = util.IntPtr(2)
+	customDS2.QStringIgnore = util.IntPtr(0)
+	customDS2.RangeRequestHandling = util.IntPtr(0)
+	customDS2.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS2.TenantID = util.IntPtr(1)
+	customDS2.TypeID = util.IntPtr(types[0].ID)
+	customDS2.XMLID = util.StrPtr("dsID2")
+	customDS2.MaxRequestHeaderBytes = nil
+
+	ds2, _, err := TOSession.CreateDeliveryService(customDS2)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ds2.CDNName = &cdn.Name
+	_, _, err = TOSession.GenerateSSLKeysForDS(*ds2.XMLID, *ds2.CDNName, tc.SSLKeyRequestFields{
+		BusinessUnit: util.StrPtr("BU"),
+		City:         util.StrPtr("CI"),
+		Organization: util.StrPtr("OR"),
+		HostName:     util.StrPtr("*.test2.com"),
+		Country:      util.StrPtr("CO"),
+		State:        util.StrPtr("ST"),
+	})
+	if err != nil {
+		t.Fatalf("unable to generate sslkeys for DS %v: %v", *customDS2.XMLID, err)
+	}
+
+	tries := 0
+	var cdnKeys []tc.CDNSSLKeys
+	for tries < 5 {
+		time.Sleep(time.Second)
+		cdnKeys, _, err = TOSession.GetCDNSSLKeys(cdn.Name, nil)
+		if err == nil && len(cdnKeys) != 0 {
+			break
+		}
+		tries++
+	}
+	if err != nil {
+		t.Fatalf("unable to get CDN %v SSL keys: %v", cdn.Name, err)
+	}
+	if len(cdnKeys) != 2 {
+		t.Errorf("expected two ssl keys for CDN %v, got %d instead", cdn.Name, len(cdnKeys))
+	}
+
+	_, err = TOSession.DeleteDeliveryService(*ds2.ID)
+	if err != nil {
+		t.Errorf("could not delete delivery service %v", *ds.XMLID)
+	}
+	_, _, err = TOSession.SnapshotCRConfigByID(cdn.ID)
+	if err != nil {
+		t.Fatalf("couldn't snap CDN: %v", err)
+	}
+	tries = 0
+	var newCdnKeys []tc.CDNSSLKeys
+	for tries < 5 {
+		time.Sleep(time.Second)
+		newCdnKeys, _, err = TOSession.GetCDNSSLKeys(cdn.Name, nil)
+		tries++
+	}
+	if err != nil {
+		t.Fatalf("unable to get CDN %v SSL keys: %v", cdn.Name, err)
+	}
+	if len(newCdnKeys) != 1 {
+		t.Errorf("expected 1 ssl keys for CDN %v, got %d instead", cdn.Name, len(newCdnKeys))
+	}
+}
+
+func DeliveryServiceSSLKeys(t *testing.T) {
+	cdn := createBlankCDN("sslkeytransfer2", t)
+
+	types, _, err := TOSession.GetTypeByName("HTTP", nil)
+	if err != nil {
+		t.Fatal("unable to get types: " + err.Error())
+	}
+	if len(types) < 1 {
+		t.Fatal("expected at least one type")
+	}
+
+	customDS := tc.DeliveryServiceV4{}
+	customDS.Active = util.BoolPtr(true)
+	customDS.CDNID = util.IntPtr(cdn.ID)
+	customDS.DSCP = util.IntPtr(0)
+	customDS.DisplayName = util.StrPtr("displayName2")
+	customDS.RoutingName = util.StrPtr("routingName2")
+	customDS.GeoLimit = util.IntPtr(0)
+	customDS.GeoProvider = util.IntPtr(0)
+	customDS.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS.InitialDispersion = util.IntPtr(1)
+	customDS.LogsEnabled = util.BoolPtr(true)
+	customDS.MissLat = util.FloatPtr(0)
+	customDS.MissLong = util.FloatPtr(0)
+	customDS.MultiSiteOrigin = util.BoolPtr(false)
+	customDS.OrgServerFQDN = util.StrPtr("https://test2.com")
+	customDS.Protocol = util.IntPtr(2)
+	customDS.QStringIgnore = util.IntPtr(0)
+	customDS.RangeRequestHandling = util.IntPtr(0)
+	customDS.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS.TenantID = util.IntPtr(1)
+	customDS.TypeID = util.IntPtr(types[0].ID)
+	customDS.XMLID = util.StrPtr("dsID2")
+	customDS.MaxRequestHeaderBytes = nil
+
+	ds, _, err := TOSession.CreateDeliveryService(customDS)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ds.CDNName = &cdn.Name
+	_, _, err = TOSession.GenerateSSLKeysForDS(*ds.XMLID, *ds.CDNName, tc.SSLKeyRequestFields{

Review comment:
       nothing to do here, just an observation: I have no idea why this method would need to take a CDN name. The CDN name can be inferred by the provided DS. I'll open a ticket to follow up on this.

##########
File path: traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go
##########
@@ -100,24 +101,149 @@ func (p *Postgres) commitTransaction(tx *sqlx.Tx, ctx context.Context, cancelFun
 	cancelFunc()
 }
 
+// GetDeliveryServiceSSLKeys retrieves the SSL keys of the given version for
+// the delivery service identified by the given xmlID. If version is empty,
+// the implementation should return the latest version.
 func (p *Postgres) GetDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) (tc.DeliveryServiceSSLKeysV15, bool, error) {
-	return tc.DeliveryServiceSSLKeysV15{}, false, notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	var jsonKeys string
+	query := "SELECT data FROM sslkey WHERE deliveryservice=$1"
+	if version != "" {
+		query += " AND version=$2"
+		err = tvTx.QueryRow(query, xmlID, version).Scan(&jsonKeys)
+	} else {
+		err = tvTx.QueryRow(query, xmlID).Scan(&jsonKeys)
+	}
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return tc.DeliveryServiceSSLKeysV15{}, false, nil
+		}
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing SELECT SSL Keys query", err, ctx.Err())
+		return tc.DeliveryServiceSSLKeysV15{}, false, e
+	}
+	sslKey := tc.DeliveryServiceSSLKeysV15{}
+	err = json.Unmarshal([]byte(jsonKeys), &sslKey)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, errors.New("unmarshalling ssl keys: " + err.Error())
+	}
+	return sslKey, true, nil
 }
 
+// PutDeliveryServiceSSLKeys stores the given SSL keys for a delivery service.
 func (p *Postgres) PutDeliveryServiceSSLKeys(key tc.DeliveryServiceSSLKeys, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+
+	keyJSON, err := json.Marshal(&key)
+	if err != nil {
+		return errors.New("marshalling keys: " + err.Error())
+	}
+
+	// delete the old ssl keys first
+	_, err = tvTx.Exec("DELETE FROM sslkey WHERE deliveryservice=$1", key.DeliveryService)
+	if err != nil && err != sql.ErrNoRows {
+		return err
+	}
+	res, err := tvTx.Exec("INSERT INTO sslkey (deliveryservice, data, cdn, version) VALUES ($1, $2, $3, $4)", key.DeliveryService, keyJSON, key.CDN, key.Version)
+	if err != nil {
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing INSERT SSL Key query", err, ctx.Err())
+		return e
+	}
+	if rowsAffected, err := res.RowsAffected(); err != nil {
+		return err
+	} else if rowsAffected == 0 {
+		return errors.New("SSL Key: no keys were inserted")
+	}
+	return nil
 }
 
+// DeleteDeliveryServiceSSLKeys removes the SSL keys of the given version (or latest
+// if version is empty) for the delivery service identified by the given xmlID.
 func (p *Postgres) DeleteDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	query := "DELETE FROM sslkey WHERE deliveryservice=$1"
+	if version != "" {

Review comment:
       Callers of this method are free to leave `version` empty, but it should then be set to `"latest"` by default in this method.

##########
File path: traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go
##########
@@ -100,24 +101,149 @@ func (p *Postgres) commitTransaction(tx *sqlx.Tx, ctx context.Context, cancelFun
 	cancelFunc()
 }
 
+// GetDeliveryServiceSSLKeys retrieves the SSL keys of the given version for
+// the delivery service identified by the given xmlID. If version is empty,
+// the implementation should return the latest version.
 func (p *Postgres) GetDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) (tc.DeliveryServiceSSLKeysV15, bool, error) {
-	return tc.DeliveryServiceSSLKeysV15{}, false, notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	var jsonKeys string
+	query := "SELECT data FROM sslkey WHERE deliveryservice=$1"
+	if version != "" {

Review comment:
       Yeah, `if version == ""` we should set it to `"latest"` and get that from the DB, but callers of this method should still be free to leave `version` empty.




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

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



[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5812: Implement SSLKeys methods for PostgreSQL Traffic Vault backend

Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5812:
URL: https://github.com/apache/trafficcontrol/pull/5812#discussion_r628532855



##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -30,6 +30,13 @@ const DefaultMaxRequestHeaderBytes = 0
 const MinRangeSliceBlockSize = 262144   // 265Kib
 const MaxRangeSliceBlockSize = 33554432 // 32Mib
 
+// SSLKeysAddResponse is a struct to store the response of addition of ssl keys for a DS,
+// along with any alert messages
+type SSLKeysAddResponse struct {

Review comment:
       This should probably go in `deliveryservice_ssl_keys.go` instead and it looks like it can just use this type alias:
   ```
   type SSLKeysAddResponse DeliveryServiceSSLKeysGenerationResponse
   ```
   since `DeliveryServiceSSLKeysGenerationResponse` is the same.
   
   EDIT: `DeliveryServiceSSLKeysGenerationResponse` doesn't exist yet (it's in another PR I had checked out), so we should just move this struct to the other file.




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

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



[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5812: Implement SSLKeys methods for PostgreSQL Traffic Vault backend

Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5812:
URL: https://github.com/apache/trafficcontrol/pull/5812#discussion_r628538485



##########
File path: traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go
##########
@@ -140,18 +140,31 @@ func (p *Postgres) PutDeliveryServiceSSLKeys(key tc.DeliveryServiceSSLKeys, tx *
 		return err
 	}
 	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
-
 	keyJSON, err := json.Marshal(&key)
 	if err != nil {
 		return errors.New("marshalling keys: " + err.Error())
 	}
 
 	// delete the old ssl keys first
-	_, err = tvTx.Exec("DELETE FROM sslkey WHERE deliveryservice=$1", key.DeliveryService)
+	_, err = tvTx.Exec("DELETE FROM sslkey WHERE deliveryservice=$1 and version=$2", key.DeliveryService, strconv.FormatInt(int64(key.Version), 10))
 	if err != nil && err != sql.ErrNoRows {
 		return err
 	}
-	res, err := tvTx.Exec("INSERT INTO sslkey (deliveryservice, data, cdn, version) VALUES ($1, $2, $3, $4)", key.DeliveryService, keyJSON, key.CDN, key.Version)
+	_, err = tvTx.Exec("DELETE FROM sslkey WHERE deliveryservice=$1 and version=$2", key.DeliveryService, latestVersion)
+	if err != nil && err != sql.ErrNoRows {
+		return err
+	}
+	res, err := tvTx.Exec("INSERT INTO sslkey (deliveryservice, data, cdn, version) VALUES ($1, $2, $3, $4)", key.DeliveryService, keyJSON, key.CDN, strconv.FormatInt(int64(key.Version), 10))
+	if err != nil {
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing INSERT SSL Key query", err, ctx.Err())
+		return e
+	}
+	if rowsAffected, err := res.RowsAffected(); err != nil {
+		return err
+	} else if rowsAffected == 0 {
+		return errors.New("SSL Key: no keys were inserted")
+	}
+	res, err = tvTx.Exec("INSERT INTO sslkey (deliveryservice, data, cdn, version) VALUES ($1, $2, $3, $4)", key.DeliveryService, keyJSON, key.CDN, latestVersion)

Review comment:
       we should combine these `INSERT`s into a single query

##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -30,6 +30,13 @@ const DefaultMaxRequestHeaderBytes = 0
 const MinRangeSliceBlockSize = 262144   // 265Kib
 const MaxRangeSliceBlockSize = 33554432 // 32Mib
 
+// SSLKeysAddResponse is a struct to store the response of addition of ssl keys for a DS,
+// along with any alert messages
+type SSLKeysAddResponse struct {

Review comment:
       This should probably go in `deliveryservice_ssl_keys.go` instead and it looks like it can just use this type alias:
   ```
   type SSLKeysAddResponse DeliveryServiceSSLKeysGenerationResponse
   ```
   since `DeliveryServiceSSLKeysGenerationResponse` is the same.

##########
File path: traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go
##########
@@ -140,18 +140,31 @@ func (p *Postgres) PutDeliveryServiceSSLKeys(key tc.DeliveryServiceSSLKeys, tx *
 		return err
 	}
 	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
-
 	keyJSON, err := json.Marshal(&key)
 	if err != nil {
 		return errors.New("marshalling keys: " + err.Error())
 	}
 
 	// delete the old ssl keys first
-	_, err = tvTx.Exec("DELETE FROM sslkey WHERE deliveryservice=$1", key.DeliveryService)
+	_, err = tvTx.Exec("DELETE FROM sslkey WHERE deliveryservice=$1 and version=$2", key.DeliveryService, strconv.FormatInt(int64(key.Version), 10))
 	if err != nil && err != sql.ErrNoRows {
 		return err
 	}
-	res, err := tvTx.Exec("INSERT INTO sslkey (deliveryservice, data, cdn, version) VALUES ($1, $2, $3, $4)", key.DeliveryService, keyJSON, key.CDN, key.Version)
+	_, err = tvTx.Exec("DELETE FROM sslkey WHERE deliveryservice=$1 and version=$2", key.DeliveryService, latestVersion)
+	if err != nil && err != sql.ErrNoRows {
+		return err
+	}

Review comment:
       we should combine these `DELETE`s into a single query




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

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



[GitHub] [trafficcontrol] mattjackson220 commented on a change in pull request #5812: Implement SSLKeys methods for PostgreSQL Traffic Vault backend

Posted by GitBox <gi...@apache.org>.
mattjackson220 commented on a change in pull request #5812:
URL: https://github.com/apache/trafficcontrol/pull/5812#discussion_r627671385



##########
File path: traffic_ops/testing/api/v4/deliveryservices_test.go
##########
@@ -171,13 +173,278 @@ func cleanUp(t *testing.T, ds tc.DeliveryServiceV4, oldCDNID int, newCDNID int)
 	if err != nil {
 		t.Error(err)
 	}
-	_, _, err = TOSession.DeleteCDN(oldCDNID)
+	if oldCDNID != -1 {
+		_, _, err = TOSession.DeleteCDN(oldCDNID)
+		if err != nil {
+			t.Error(err)
+		}
+	}
+	if newCDNID != -1 {
+		_, _, err = TOSession.DeleteCDN(newCDNID)
+		if err != nil {
+			t.Error(err)
+		}
+	}
+}
+
+func DeleteCDNOldSSLKeys(t *testing.T) {
+	cdn := createBlankCDN("sslkeytransfer", t)
+
+	types, _, err := TOSession.GetTypeByName("HTTP", nil)
 	if err != nil {
-		t.Error(err)
+		t.Fatal("unable to get types: " + err.Error())
+	}
+	if len(types) < 1 {
+		t.Fatal("expected at least one type")
 	}
-	_, _, err = TOSession.DeleteCDN(newCDNID)
+
+	customDS := tc.DeliveryServiceV4{}
+	customDS.Active = util.BoolPtr(true)
+	customDS.CDNID = util.IntPtr(cdn.ID)
+	customDS.DSCP = util.IntPtr(0)
+	customDS.DisplayName = util.StrPtr("displayName")
+	customDS.RoutingName = util.StrPtr("routingName")
+	customDS.GeoLimit = util.IntPtr(0)
+	customDS.GeoProvider = util.IntPtr(0)
+	customDS.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS.InitialDispersion = util.IntPtr(1)
+	customDS.LogsEnabled = util.BoolPtr(true)
+	customDS.MissLat = util.FloatPtr(0)
+	customDS.MissLong = util.FloatPtr(0)
+	customDS.MultiSiteOrigin = util.BoolPtr(false)
+	customDS.OrgServerFQDN = util.StrPtr("https://test.com")
+	customDS.Protocol = util.IntPtr(2)
+	customDS.QStringIgnore = util.IntPtr(0)
+	customDS.RangeRequestHandling = util.IntPtr(0)
+	customDS.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS.TenantID = util.IntPtr(1)
+	customDS.TypeID = util.IntPtr(types[0].ID)
+	customDS.XMLID = util.StrPtr("dsID")
+	customDS.MaxRequestHeaderBytes = nil
+
+	ds, _, err := TOSession.CreateDeliveryService(customDS)
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
+	}
+	ds.CDNName = &cdn.Name
+	_, _, err = TOSession.GenerateSSLKeysForDS(*ds.XMLID, *ds.CDNName, tc.SSLKeyRequestFields{
+		BusinessUnit: util.StrPtr("BU"),
+		City:         util.StrPtr("CI"),
+		Organization: util.StrPtr("OR"),
+		HostName:     util.StrPtr("*.test.com"),
+		Country:      util.StrPtr("CO"),
+		State:        util.StrPtr("ST"),
+	})
+	if err != nil {
+		t.Fatalf("unable to generate sslkeys for DS %v: %v", *customDS.XMLID, err)
+	}
+	defer cleanUp(t, ds, cdn.ID, -1)
+
+	// Second DS creation
+	customDS2 := tc.DeliveryServiceV4{}
+	customDS2.Active = util.BoolPtr(true)
+	customDS2.CDNID = util.IntPtr(cdn.ID)
+	customDS2.DSCP = util.IntPtr(0)
+	customDS2.DisplayName = util.StrPtr("displayName2")
+	customDS2.RoutingName = util.StrPtr("routingName2")
+	customDS2.GeoLimit = util.IntPtr(0)
+	customDS2.GeoProvider = util.IntPtr(0)
+	customDS2.IPV6RoutingEnabled = util.BoolPtr(false)
+	customDS2.InitialDispersion = util.IntPtr(1)
+	customDS2.LogsEnabled = util.BoolPtr(true)
+	customDS2.MissLat = util.FloatPtr(0)
+	customDS2.MissLong = util.FloatPtr(0)
+	customDS2.MultiSiteOrigin = util.BoolPtr(false)
+	customDS2.OrgServerFQDN = util.StrPtr("https://test2.com")
+	customDS2.Protocol = util.IntPtr(2)
+	customDS2.QStringIgnore = util.IntPtr(0)
+	customDS2.RangeRequestHandling = util.IntPtr(0)
+	customDS2.RegionalGeoBlocking = util.BoolPtr(false)
+	customDS2.TenantID = util.IntPtr(1)
+	customDS2.TypeID = util.IntPtr(types[0].ID)
+	customDS2.XMLID = util.StrPtr("dsID2")
+	customDS2.MaxRequestHeaderBytes = nil

Review comment:
       maybe pull these and the ones below into a getCustomDs func or something to cut down on repeated lines?

##########
File path: traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go
##########
@@ -100,24 +101,149 @@ func (p *Postgres) commitTransaction(tx *sqlx.Tx, ctx context.Context, cancelFun
 	cancelFunc()
 }
 
+// GetDeliveryServiceSSLKeys retrieves the SSL keys of the given version for
+// the delivery service identified by the given xmlID. If version is empty,
+// the implementation should return the latest version.
 func (p *Postgres) GetDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) (tc.DeliveryServiceSSLKeysV15, bool, error) {
-	return tc.DeliveryServiceSSLKeysV15{}, false, notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	var jsonKeys string
+	query := "SELECT data FROM sslkey WHERE deliveryservice=$1"
+	if version != "" {

Review comment:
       we should require version to be set otherwise we will get a list of old SSL keys

##########
File path: traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go
##########
@@ -100,24 +101,149 @@ func (p *Postgres) commitTransaction(tx *sqlx.Tx, ctx context.Context, cancelFun
 	cancelFunc()
 }
 
+// GetDeliveryServiceSSLKeys retrieves the SSL keys of the given version for
+// the delivery service identified by the given xmlID. If version is empty,
+// the implementation should return the latest version.
 func (p *Postgres) GetDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) (tc.DeliveryServiceSSLKeysV15, bool, error) {
-	return tc.DeliveryServiceSSLKeysV15{}, false, notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	var jsonKeys string
+	query := "SELECT data FROM sslkey WHERE deliveryservice=$1"
+	if version != "" {
+		query += " AND version=$2"
+		err = tvTx.QueryRow(query, xmlID, version).Scan(&jsonKeys)
+	} else {
+		err = tvTx.QueryRow(query, xmlID).Scan(&jsonKeys)
+	}
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return tc.DeliveryServiceSSLKeysV15{}, false, nil
+		}
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing SELECT SSL Keys query", err, ctx.Err())
+		return tc.DeliveryServiceSSLKeysV15{}, false, e
+	}
+	sslKey := tc.DeliveryServiceSSLKeysV15{}
+	err = json.Unmarshal([]byte(jsonKeys), &sslKey)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, errors.New("unmarshalling ssl keys: " + err.Error())
+	}
+	return sslKey, true, nil
 }
 
+// PutDeliveryServiceSSLKeys stores the given SSL keys for a delivery service.
 func (p *Postgres) PutDeliveryServiceSSLKeys(key tc.DeliveryServiceSSLKeys, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+
+	keyJSON, err := json.Marshal(&key)
+	if err != nil {
+		return errors.New("marshalling keys: " + err.Error())
+	}
+
+	// delete the old ssl keys first
+	_, err = tvTx.Exec("DELETE FROM sslkey WHERE deliveryservice=$1", key.DeliveryService)
+	if err != nil && err != sql.ErrNoRows {
+		return err
+	}
+	res, err := tvTx.Exec("INSERT INTO sslkey (deliveryservice, data, cdn, version) VALUES ($1, $2, $3, $4)", key.DeliveryService, keyJSON, key.CDN, key.Version)
+	if err != nil {
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing INSERT SSL Key query", err, ctx.Err())
+		return e
+	}
+	if rowsAffected, err := res.RowsAffected(); err != nil {
+		return err
+	} else if rowsAffected == 0 {
+		return errors.New("SSL Key: no keys were inserted")
+	}
+	return nil
 }
 
+// DeleteDeliveryServiceSSLKeys removes the SSL keys of the given version (or latest
+// if version is empty) for the delivery service identified by the given xmlID.
 func (p *Postgres) DeleteDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	query := "DELETE FROM sslkey WHERE deliveryservice=$1"
+	if version != "" {

Review comment:
       we should require version in this query otherwise delete the latest. if version isnt provided as it is now then it will delete all ssl keys for that ds

##########
File path: traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go
##########
@@ -100,24 +101,149 @@ func (p *Postgres) commitTransaction(tx *sqlx.Tx, ctx context.Context, cancelFun
 	cancelFunc()
 }
 
+// GetDeliveryServiceSSLKeys retrieves the SSL keys of the given version for
+// the delivery service identified by the given xmlID. If version is empty,
+// the implementation should return the latest version.
 func (p *Postgres) GetDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) (tc.DeliveryServiceSSLKeysV15, bool, error) {
-	return tc.DeliveryServiceSSLKeysV15{}, false, notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	var jsonKeys string
+	query := "SELECT data FROM sslkey WHERE deliveryservice=$1"
+	if version != "" {
+		query += " AND version=$2"
+		err = tvTx.QueryRow(query, xmlID, version).Scan(&jsonKeys)
+	} else {
+		err = tvTx.QueryRow(query, xmlID).Scan(&jsonKeys)
+	}
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return tc.DeliveryServiceSSLKeysV15{}, false, nil
+		}
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing SELECT SSL Keys query", err, ctx.Err())
+		return tc.DeliveryServiceSSLKeysV15{}, false, e
+	}
+	sslKey := tc.DeliveryServiceSSLKeysV15{}
+	err = json.Unmarshal([]byte(jsonKeys), &sslKey)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, errors.New("unmarshalling ssl keys: " + err.Error())
+	}
+	return sslKey, true, nil
 }
 
+// PutDeliveryServiceSSLKeys stores the given SSL keys for a delivery service.
 func (p *Postgres) PutDeliveryServiceSSLKeys(key tc.DeliveryServiceSSLKeys, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+
+	keyJSON, err := json.Marshal(&key)

Review comment:
       we should check to make sure it has a version provided and if it doesnt then read the db and increase from the latest

##########
File path: traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go
##########
@@ -100,24 +101,149 @@ func (p *Postgres) commitTransaction(tx *sqlx.Tx, ctx context.Context, cancelFun
 	cancelFunc()
 }
 
+// GetDeliveryServiceSSLKeys retrieves the SSL keys of the given version for
+// the delivery service identified by the given xmlID. If version is empty,
+// the implementation should return the latest version.
 func (p *Postgres) GetDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) (tc.DeliveryServiceSSLKeysV15, bool, error) {
-	return tc.DeliveryServiceSSLKeysV15{}, false, notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	var jsonKeys string
+	query := "SELECT data FROM sslkey WHERE deliveryservice=$1"
+	if version != "" {
+		query += " AND version=$2"
+		err = tvTx.QueryRow(query, xmlID, version).Scan(&jsonKeys)
+	} else {
+		err = tvTx.QueryRow(query, xmlID).Scan(&jsonKeys)
+	}
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return tc.DeliveryServiceSSLKeysV15{}, false, nil
+		}
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing SELECT SSL Keys query", err, ctx.Err())
+		return tc.DeliveryServiceSSLKeysV15{}, false, e
+	}
+	sslKey := tc.DeliveryServiceSSLKeysV15{}
+	err = json.Unmarshal([]byte(jsonKeys), &sslKey)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, errors.New("unmarshalling ssl keys: " + err.Error())
+	}
+	return sslKey, true, nil
 }
 
+// PutDeliveryServiceSSLKeys stores the given SSL keys for a delivery service.
 func (p *Postgres) PutDeliveryServiceSSLKeys(key tc.DeliveryServiceSSLKeys, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+
+	keyJSON, err := json.Marshal(&key)
+	if err != nil {
+		return errors.New("marshalling keys: " + err.Error())
+	}
+
+	// delete the old ssl keys first
+	_, err = tvTx.Exec("DELETE FROM sslkey WHERE deliveryservice=$1", key.DeliveryService)
+	if err != nil && err != sql.ErrNoRows {
+		return err
+	}
+	res, err := tvTx.Exec("INSERT INTO sslkey (deliveryservice, data, cdn, version) VALUES ($1, $2, $3, $4)", key.DeliveryService, keyJSON, key.CDN, key.Version)
+	if err != nil {
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing INSERT SSL Key query", err, ctx.Err())
+		return e
+	}
+	if rowsAffected, err := res.RowsAffected(); err != nil {
+		return err
+	} else if rowsAffected == 0 {
+		return errors.New("SSL Key: no keys were inserted")
+	}
+	return nil
 }
 
+// DeleteDeliveryServiceSSLKeys removes the SSL keys of the given version (or latest
+// if version is empty) for the delivery service identified by the given xmlID.
 func (p *Postgres) DeleteDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	query := "DELETE FROM sslkey WHERE deliveryservice=$1"
+	if version != "" {
+		query += " AND version=$2"
+		_, err = tvTx.Exec(query, xmlID, version)
+	} else {
+		_, err = tvTx.Exec(query, xmlID)
+	}
+	if err != nil {
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing DELETE SSL Key query", err, ctx.Err())
+		return e
+	}
+	return nil
 }
 
+// DeleteOldDeliveryServiceSSLKeys takes a set of existingXMLIDs as input and will remove
+// all SSL keys for delivery services in the CDN identified by the given cdnName that
+// do not contain an xmlID in the given set of existingXMLIDs. This method is called
+// during a snapshot operation in order to delete SSL keys for delivery services that
+// no longer exist.
 func (p *Postgres) DeleteOldDeliveryServiceSSLKeys(existingXMLIDs map[string]struct{}, cdnName string, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	var keys []string
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+
+	if len(existingXMLIDs) == 0 {
+		keys = append(keys, "")
+	}
+	for k, _ := range existingXMLIDs {
+		keys = append(keys, k)
+	}
+	_, err = tvTx.Exec("DELETE FROM sslkey WHERE cdn=$1 AND deliveryservice <> ALL ($2)", cdnName, pq.Array(keys))
+	if err != nil {
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing DELETE OLD SSL Key query", err, ctx.Err())
+		return e
+	}
+	return nil
 }
 
+// GetCDNSSLKeys retrieves all the SSL keys for delivery services in the CDN identified
+// by the given cdnName.
 func (p *Postgres) GetCDNSSLKeys(cdnName string, tx *sql.Tx, ctx context.Context) ([]tc.CDNSSLKey, error) {
-	return nil, notImplementedErr
+	var keys []tc.CDNSSLKey
+	var key tc.CDNSSLKey
+
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return keys, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+
+	rows, err := tvTx.Query("SELECT data from sslkey WHERE cdn=$1", cdnName)

Review comment:
       we will also want to filter this query to only get the latest keys for each ds

##########
File path: traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go
##########
@@ -100,24 +101,149 @@ func (p *Postgres) commitTransaction(tx *sqlx.Tx, ctx context.Context, cancelFun
 	cancelFunc()
 }
 
+// GetDeliveryServiceSSLKeys retrieves the SSL keys of the given version for
+// the delivery service identified by the given xmlID. If version is empty,
+// the implementation should return the latest version.
 func (p *Postgres) GetDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) (tc.DeliveryServiceSSLKeysV15, bool, error) {
-	return tc.DeliveryServiceSSLKeysV15{}, false, notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	var jsonKeys string
+	query := "SELECT data FROM sslkey WHERE deliveryservice=$1"
+	if version != "" {
+		query += " AND version=$2"
+		err = tvTx.QueryRow(query, xmlID, version).Scan(&jsonKeys)
+	} else {
+		err = tvTx.QueryRow(query, xmlID).Scan(&jsonKeys)
+	}
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return tc.DeliveryServiceSSLKeysV15{}, false, nil
+		}
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing SELECT SSL Keys query", err, ctx.Err())
+		return tc.DeliveryServiceSSLKeysV15{}, false, e
+	}
+	sslKey := tc.DeliveryServiceSSLKeysV15{}
+	err = json.Unmarshal([]byte(jsonKeys), &sslKey)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, errors.New("unmarshalling ssl keys: " + err.Error())
+	}
+	return sslKey, true, nil
 }
 
+// PutDeliveryServiceSSLKeys stores the given SSL keys for a delivery service.
 func (p *Postgres) PutDeliveryServiceSSLKeys(key tc.DeliveryServiceSSLKeys, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+
+	keyJSON, err := json.Marshal(&key)
+	if err != nil {
+		return errors.New("marshalling keys: " + err.Error())
+	}
+
+	// delete the old ssl keys first
+	_, err = tvTx.Exec("DELETE FROM sslkey WHERE deliveryservice=$1", key.DeliveryService)

Review comment:
       i dont think we want to delete the old keys. we just want to up the version and save that out. at least i believe thats how Riak worked




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

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



[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5812: Implement SSLKeys methods for PostgreSQL Traffic Vault backend

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5812:
URL: https://github.com/apache/trafficcontrol/pull/5812#discussion_r627859905



##########
File path: traffic_ops/v4-client/deliveryservice.go
##########
@@ -289,6 +292,18 @@ func (to *Session) GenerateSSLKeysForDS(XMLID string, CDNName string, sslFields
 	return response.Response, reqInf, nil
 }
 
+// AddSSLKeysForDS adds SSL Keys for the given DS
+func (to *Session) AddSSLKeysForDS(request tc.DeliveryServiceAddSSLKeysReq) (string, toclientlib.ReqInf, error) {

Review comment:
       I would appreciate that, because if you don't do it I most certainly will




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

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



[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5812: Implement SSLKeys methods for PostgreSQL Traffic Vault backend

Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5812:
URL: https://github.com/apache/trafficcontrol/pull/5812#discussion_r628589453



##########
File path: traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go
##########
@@ -100,24 +103,147 @@ func (p *Postgres) commitTransaction(tx *sqlx.Tx, ctx context.Context, cancelFun
 	cancelFunc()
 }
 
+// GetDeliveryServiceSSLKeys retrieves the SSL keys of the given version for
+// the delivery service identified by the given xmlID. If version is empty,
+// the implementation should return the latest version.
 func (p *Postgres) GetDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) (tc.DeliveryServiceSSLKeysV15, bool, error) {
-	return tc.DeliveryServiceSSLKeysV15{}, false, notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	var jsonKeys string
+	query := "SELECT data FROM sslkey WHERE deliveryservice=$1 AND version=$2"
+	if version == "" {
+		version = "latest"
+	}
+	err = tvTx.QueryRow(query, xmlID, version).Scan(&jsonKeys)
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return tc.DeliveryServiceSSLKeysV15{}, false, nil
+		}
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing SELECT SSL Keys query", err, ctx.Err())
+		return tc.DeliveryServiceSSLKeysV15{}, false, e
+	}
+	sslKey := tc.DeliveryServiceSSLKeysV15{}
+	err = json.Unmarshal([]byte(jsonKeys), &sslKey)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, errors.New("unmarshalling ssl keys: " + err.Error())
+	}
+	return sslKey, true, nil
 }
 
+// PutDeliveryServiceSSLKeys stores the given SSL keys for a delivery service.
 func (p *Postgres) PutDeliveryServiceSSLKeys(key tc.DeliveryServiceSSLKeys, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	keyJSON, err := json.Marshal(&key)
+	if err != nil {
+		return errors.New("marshalling keys: " + err.Error())
+	}
+
+	// delete the old ssl keys first
+	oldVersions := []string{strconv.FormatInt(int64(key.Version), 10), latestVersion}
+	_, err = tvTx.Exec("DELETE FROM sslkey WHERE deliveryservice=$1 and version=ANY($2)", key.DeliveryService, pq.Array(oldVersions))
+	if err != nil && err != sql.ErrNoRows {
+		return err

Review comment:
       before returning we should use `checkErrWithContext` here in case the error is due to context cancellation

##########
File path: traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go
##########
@@ -100,24 +103,147 @@ func (p *Postgres) commitTransaction(tx *sqlx.Tx, ctx context.Context, cancelFun
 	cancelFunc()
 }
 
+// GetDeliveryServiceSSLKeys retrieves the SSL keys of the given version for
+// the delivery service identified by the given xmlID. If version is empty,
+// the implementation should return the latest version.
 func (p *Postgres) GetDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) (tc.DeliveryServiceSSLKeysV15, bool, error) {
-	return tc.DeliveryServiceSSLKeysV15{}, false, notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	var jsonKeys string
+	query := "SELECT data FROM sslkey WHERE deliveryservice=$1 AND version=$2"
+	if version == "" {
+		version = "latest"
+	}
+	err = tvTx.QueryRow(query, xmlID, version).Scan(&jsonKeys)
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return tc.DeliveryServiceSSLKeysV15{}, false, nil
+		}
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing SELECT SSL Keys query", err, ctx.Err())
+		return tc.DeliveryServiceSSLKeysV15{}, false, e
+	}
+	sslKey := tc.DeliveryServiceSSLKeysV15{}
+	err = json.Unmarshal([]byte(jsonKeys), &sslKey)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, errors.New("unmarshalling ssl keys: " + err.Error())
+	}
+	return sslKey, true, nil
 }
 
+// PutDeliveryServiceSSLKeys stores the given SSL keys for a delivery service.
 func (p *Postgres) PutDeliveryServiceSSLKeys(key tc.DeliveryServiceSSLKeys, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	keyJSON, err := json.Marshal(&key)
+	if err != nil {
+		return errors.New("marshalling keys: " + err.Error())
+	}
+
+	// delete the old ssl keys first
+	oldVersions := []string{strconv.FormatInt(int64(key.Version), 10), latestVersion}
+	_, err = tvTx.Exec("DELETE FROM sslkey WHERE deliveryservice=$1 and version=ANY($2)", key.DeliveryService, pq.Array(oldVersions))
+	if err != nil && err != sql.ErrNoRows {
+		return err
+	}
+
+	// insert the new ssl keys now
+	res, err := tvTx.Exec("INSERT INTO sslkey (deliveryservice, data, cdn, version) VALUES ($1, $2, $3, $4), ($5, $6, $7, $8)", key.DeliveryService, keyJSON, key.CDN, strconv.FormatInt(int64(key.Version), 10), key.DeliveryService, keyJSON, key.CDN, latestVersion)
+	if err != nil {
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing INSERT SSL Key query", err, ctx.Err())
+		return e
+	}
+	if rowsAffected, err := res.RowsAffected(); err != nil {
+		return err
+	} else if rowsAffected == 0 {
+		return errors.New("SSL Key: no keys were inserted")
+	}
+	return nil
 }
 
+// DeleteDeliveryServiceSSLKeys removes the SSL keys of the given version (or latest
+// if version is empty) for the delivery service identified by the given xmlID.
 func (p *Postgres) DeleteDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	if version == "" {
+		version = latestVersion
+	}
+	query := "DELETE FROM sslkey WHERE deliveryservice=$1 AND version=$2"
+	_, err = tvTx.Exec(query, xmlID, version)
+	if err != nil {
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing DELETE SSL Key query", err, ctx.Err())
+		return e
+	}
+	return nil
 }
 
+// DeleteOldDeliveryServiceSSLKeys takes a set of existingXMLIDs as input and will remove
+// all SSL keys for delivery services in the CDN identified by the given cdnName that
+// do not contain an xmlID in the given set of existingXMLIDs. This method is called
+// during a snapshot operation in order to delete SSL keys for delivery services that
+// no longer exist.
 func (p *Postgres) DeleteOldDeliveryServiceSSLKeys(existingXMLIDs map[string]struct{}, cdnName string, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	var keys []string
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+
+	if len(existingXMLIDs) == 0 {
+		keys = append(keys, "")
+	}
+	for k, _ := range existingXMLIDs {
+		keys = append(keys, k)
+	}
+	_, err = tvTx.Exec("DELETE FROM sslkey WHERE cdn=$1 AND deliveryservice <> ALL ($2)", cdnName, pq.Array(keys))
+	if err != nil {
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing DELETE OLD SSL Key query", err, ctx.Err())
+		return e
+	}
+	return nil
 }
 
+// GetCDNSSLKeys retrieves all the SSL keys for delivery services in the CDN identified
+// by the given cdnName.
 func (p *Postgres) GetCDNSSLKeys(cdnName string, tx *sql.Tx, ctx context.Context) ([]tc.CDNSSLKey, error) {
-	return nil, notImplementedErr
+	var keys []tc.CDNSSLKey
+	var key tc.CDNSSLKey
+
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return keys, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+
+	rows, err := tvTx.Query("SELECT data from sslkey WHERE cdn=$1 AND version=$2", cdnName, latestVersion)
+	if err != nil {
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing GET SSL Keys for CDN query", err, ctx.Err())
+		return keys, e
+	}
+	defer rows.Close()
+	for rows.Next() {
+		jsonKey := ""
+		if err := rows.Scan(&jsonKey); err != nil {
+			return keys, errors.New("scanning cdn ssl keys: " + err.Error())

Review comment:
       before returning we should use `checkErrWithContext` here in case the error is due to context cancellation




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

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



[GitHub] [trafficcontrol] srijeet0406 commented on a change in pull request #5812: Implement SSLKeys methods for PostgreSQL Traffic Vault backend

Posted by GitBox <gi...@apache.org>.
srijeet0406 commented on a change in pull request #5812:
URL: https://github.com/apache/trafficcontrol/pull/5812#discussion_r628357412



##########
File path: traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go
##########
@@ -100,24 +101,149 @@ func (p *Postgres) commitTransaction(tx *sqlx.Tx, ctx context.Context, cancelFun
 	cancelFunc()
 }
 
+// GetDeliveryServiceSSLKeys retrieves the SSL keys of the given version for
+// the delivery service identified by the given xmlID. If version is empty,
+// the implementation should return the latest version.
 func (p *Postgres) GetDeliveryServiceSSLKeys(xmlID string, version string, tx *sql.Tx, ctx context.Context) (tc.DeliveryServiceSSLKeysV15, bool, error) {
-	return tc.DeliveryServiceSSLKeysV15{}, false, notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+	var jsonKeys string
+	query := "SELECT data FROM sslkey WHERE deliveryservice=$1"
+	if version != "" {
+		query += " AND version=$2"
+		err = tvTx.QueryRow(query, xmlID, version).Scan(&jsonKeys)
+	} else {
+		err = tvTx.QueryRow(query, xmlID).Scan(&jsonKeys)
+	}
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return tc.DeliveryServiceSSLKeysV15{}, false, nil
+		}
+		e := checkErrWithContext("Traffic Vault PostgreSQL: executing SELECT SSL Keys query", err, ctx.Err())
+		return tc.DeliveryServiceSSLKeysV15{}, false, e
+	}
+	sslKey := tc.DeliveryServiceSSLKeysV15{}
+	err = json.Unmarshal([]byte(jsonKeys), &sslKey)
+	if err != nil {
+		return tc.DeliveryServiceSSLKeysV15{}, false, errors.New("unmarshalling ssl keys: " + err.Error())
+	}
+	return sslKey, true, nil
 }
 
+// PutDeliveryServiceSSLKeys stores the given SSL keys for a delivery service.
 func (p *Postgres) PutDeliveryServiceSSLKeys(key tc.DeliveryServiceSSLKeys, tx *sql.Tx, ctx context.Context) error {
-	return notImplementedErr
+	tvTx, dbCtx, cancelFunc, err := p.beginTransaction(ctx)
+	if err != nil {
+		return err
+	}
+	defer p.commitTransaction(tvTx, dbCtx, cancelFunc)
+
+	keyJSON, err := json.Marshal(&key)

Review comment:
       So the `version` cannot be empty in this request, the validators make sure of that before passing the request body into this method. But I've made the change so that the methos actually insert two rows, one with the specified version, and one with `latest`, after deleting the `latest` and the key with the given version (if provided)




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

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