Repository: google/exposure-notifications-server
Pull request #1217 (2020-12-08 01:53:01)

make data padding more better

## Proposed Changes

* Take randomness out of export padding and just clone every key at least once
* Makes some paths less complicated, but makes many paths more complicated (see before and after)

Using the same source data:

Before:
![before](https://user-images.githubusercontent.com/92319/101428111-f45df880-38b4-11eb-9f02-617628dd38b6.png)

After
![after](https://user-images.githubusercontent.com/92319/101428120-f922ac80-38b4-11eb-84f5-709f9cb2fd51.png)

**Release Note**

```release-note
Export padding will clone every key at least once to better obscure TEK links.
```

google-oss-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/google/exposure-notifications-server/pull/1217#" title="Author self-approved">mikehelmick</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=google%2Fexposure-notifications-server).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/google/exposure-notifications-server/blob/main/OWNERS)~~ [mikehelmick]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
google-oss-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/google/exposure-notifications-server/pull/1217#" title="Author self-approved">mikehelmick</a>*, *<a href="https://github.com/google/exposure-notifications-server/pull/1217#pullrequestreview-546693627" title="Approved">whaught</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=google%2Fexposure-notifications-server).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/google/exposure-notifications-server/blob/main/OWNERS)~~ [mikehelmick,whaught]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/internal/export/worker.go b/internal/export/worker.go
index 203a882..6eaaedf 100644
--- a/internal/export/worker.go
+++ b/internal/export/worker.go
@@ -229,7 +229,7 @@ func (s *Server) batchExposures(ctx context.Context, criteria publishdatabase.It
 		// will give away the generated data.
 		lastGroup := groups[len(groups)-1]
 		var generated []*publishmodel.Exposure
-		lastGroup.exposures, generated, err = ensureMinNumExposures(lastGroup.exposures, outputRegion, s.config.MinRecords, s.config.PaddingRange, maxCreatedAt)
+		lastGroup.exposures, generated, err = ensureMinNumExposures(lastGroup.exposures, outputRegion, s.config.MinRecords, s.config.PaddingRange, maxRecords, maxCreatedAt)
 		if err != nil {
 			return nil, fmt.Errorf("ensureMinNumExposures: %w", err)
 		}
@@ -496,7 +496,7 @@ func randomInt(min, max int) (int, error) {
 	return int(n.Int64()) + min, nil
 }
 
-func ensureMinNumExposures(exposures []*publishmodel.Exposure, region string, minLength, jitter int, createdAt time.Time) ([]*publishmodel.Exposure, []*publishmodel.Exposure, error) {
+func ensureMinNumExposures(exposures []*publishmodel.Exposure, region string, minLength, jitter, maxLength int, createdAt time.Time) ([]*publishmodel.Exposure, []*publishmodel.Exposure, error) {
 	extra, _ := randomInt(0, jitter)
 	target := minLength + extra
 
@@ -504,42 +504,43 @@ func ensureMinNumExposures(exposures []*publishmodel.Exposure, region string, mi
 		return exposures, make([]*publishmodel.Exposure, 0), nil
 	}
 
-	sourceLen := len(exposures) - 1
+	sourceLen := len(exposures)
 	generated := make([]*publishmodel.Exposure, 0, target-len(exposures))
 
 	for len(exposures) < target {
-		// Pieces needed are
-		// (1) exposure key, (2) interval number, (3) transmission risk
-		// Exposure key is 16 random bytes.
-		eKey := make([]byte, verifyapi.KeyLength)
-		_, err := rand.Read(eKey)
-		if err != nil {
-			return nil, nil, fmt.Errorf("rand.Read: %w", err)
-		}
+		// loop through the source data
+		for fromIdx := 0; fromIdx < sourceLen; fromIdx++ {
+			// Pieces needed are
+			// (1) exposure key, (2) interval number, (3) transmission risk
+			// Exposure key is 16 random bytes.
+			eKey := make([]byte, verifyapi.KeyLength)
+			_, err := rand.Read(eKey)
+			if err != nil {
+				return nil, nil, fmt.Errorf("rand.Read: %w", err)
+			}
 
-		// Copy timing and report data from a key.
-		fromIdx, err := randomInt(0, sourceLen)
-		if err != nil {
-			return nil, nil, fmt.Errorf("randomInt: %w", err)
-		}
+			ek := &publishmodel.Exposure{
+				ExposureKey:           eKey,
+				TransmissionRisk:      exposures[fromIdx].TransmissionRisk,
+				AppPackageName:        exportAppPackageName,
+				Regions:               exposures[fromIdx].Regions,
+				Traveler:              exposures[fromIdx].Traveler,
+				IntervalNumber:        exposures[fromIdx].IntervalNumber,
+				IntervalCount:         exposures[fromIdx].IntervalCount,
+				CreatedAt:             createdAt,
+				LocalProvenance:       true,
+				ReportType:            exposures[fromIdx].ReportType,
+				DaysSinceSymptomOnset: exposures[fromIdx].DaysSinceSymptomOnset,
+				// key revision fields are not used here - generated data only covers primary keys.
+				// The rest of the publishmodel.Exposure fields are not used in the export file.
+			}
+			generated = append(generated, ek)
+			exposures = append(exposures, ek)
 
-		ek := &publishmodel.Exposure{
-			ExposureKey:           eKey,
-			TransmissionRisk:      exposures[fromIdx].TransmissionRisk,
-			AppPackageName:        exportAppPackageName,
-			Regions:               exposures[fromIdx].Regions,
-			Traveler:              exposures[fromIdx].Traveler,
-			IntervalNumber:        exposures[fromIdx].IntervalNumber,
-			IntervalCount:         exposures[fromIdx].IntervalCount,
-			CreatedAt:             createdAt,
-			LocalProvenance:       true,
-			ReportType:            exposures[fromIdx].ReportType,
-			DaysSinceSymptomOnset: exposures[fromIdx].DaysSinceSymptomOnset,
-			// key revision fields are not used here - generated data only covers primary keys.
-			// The rest of the publishmodel.Exposure fields are not used in the export file.
+			if len(exposures) >= maxLength {
+				return exposures, generated, nil
+			}
 		}
-		generated = append(generated, ek)
-		exposures = append(exposures, ek)
 	}
 
 	return exposures, generated, nil


Test patch:
diff --git a/internal/export/worker_test.go b/internal/export/worker_test.go
index a4fff3d..e40e029 100644
--- a/internal/export/worker_test.go
+++ b/internal/export/worker_test.go
@@ -58,7 +58,7 @@ func TestDoNotPadZeroLength(t *testing.T) {
 	t.Parallel()
 
 	exposures := make([]*publishmodel.Exposure, 0)
-	exposures, generated, err := ensureMinNumExposures(exposures, "US", 1000, 100, time.Now())
+	exposures, generated, err := ensureMinNumExposures(exposures, "US", 1000, 100, 2000, time.Now())
 	if err != nil {
 		t.Fatalf("unepected error: %v", err)
 	}
@@ -115,7 +115,7 @@ func TestEnsureMinExposures(t *testing.T) {
 
 	// pad the download.
 	inputSize := len(exposures)
-	exposures, generated, err := ensureMinNumExposures(exposures, "US", numKeys, variance, time.Now())
+	exposures, generated, err := ensureMinNumExposures(exposures, "US", numKeys, variance, numKeys*10, time.Now())
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -131,7 +131,7 @@ func TestEnsureMinExposures(t *testing.T) {
 	}
 	for k, v := range m {
 		if v < 20 {
-			t.Errorf("distribution not random, expected >= 30 keys with start interval %v, got %v", k, v)
+			t.Errorf("distribution not random, expected >= 20 keys with start interval %v, got %v", k, v)
 		}
 	}
 }