Skip to content

Commit 87a6155

Browse files
phbnfphbnf
committed
address comments
1 parent 95017b4 commit 87a6155

File tree

2 files changed

+38
-38
lines changed

2 files changed

+38
-38
lines changed

‎internal/scti/handlers_test.go

Lines changed: 30 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -321,38 +321,36 @@ func TestNewPathHandlers(t *testing.T) {
321321

322322
func TestGetRoots(t *testing.T) {
323323
log := setupTestLog(t)
324-
server := setupTestServer(t, log, path.Join(prefix, "ct/v1/get-roots"))
324+
server := setupTestServer(t, log, path.Join(prefix, types.GetRootsPath))
325325
defer server.Close()
326326

327-
t.Run("get-roots", func(t *testing.T) {
328-
resp, err := http.Get(server.URL + path.Join(prefix, "ct/v1/get-roots"))
329-
if err != nil {
330-
t.Fatalf("Failed to get roots: %v", err)
331-
}
327+
resp, err := http.Get(server.URL + path.Join(prefix, types.GetRootsPath))
328+
if err != nil {
329+
t.Fatalf("Failed to get roots: %v", err)
330+
}
332331

333-
if resp.StatusCode != http.StatusOK {
334-
t.Errorf("Unexpected status code: %v", resp.StatusCode)
335-
}
332+
if resp.StatusCode != http.StatusOK {
333+
t.Errorf("Unexpected status code: %v", resp.StatusCode)
334+
}
336335

337-
var roots types.GetRootsResponse
338-
err = json.NewDecoder(resp.Body).Decode(&roots)
339-
if err != nil {
340-
t.Errorf("Failed to decode response: %v", err)
341-
}
336+
var roots types.GetRootsResponse
337+
err = json.NewDecoder(resp.Body).Decode(&roots)
338+
if err != nil {
339+
t.Errorf("Failed to decode response: %v", err)
340+
}
342341

343-
if got, want := len(roots.Certificates), 1; got != want {
344-
t.Errorf("Unexpected number of certificates: got %d, want %d", got, want)
345-
}
342+
if got, want := len(roots.Certificates), 1; got != want {
343+
t.Errorf("Unexpected number of certificates: got %d, want %d", got, want)
344+
}
346345

347-
got, err := base64.StdEncoding.DecodeString(roots.Certificates[0])
348-
if err != nil {
349-
t.Errorf("Failed to decode certificate: %v", err)
350-
}
351-
want, _ := pem.Decode([]byte(testdata.CACertPEM))
352-
if !bytes.Equal(got, want.Bytes) {
353-
t.Errorf("Unexpected root: got %s, want %s", roots.Certificates[0], base64.StdEncoding.EncodeToString(want.Bytes))
354-
}
355-
})
346+
got, err := base64.StdEncoding.DecodeString(roots.Certificates[0])
347+
if err != nil {
348+
t.Errorf("Failed to decode certificate: %v", err)
349+
}
350+
want, _ := pem.Decode([]byte(testdata.CACertPEM))
351+
if !bytes.Equal(got, want.Bytes) {
352+
t.Errorf("Unexpected root: got %s, want %s", roots.Certificates[0], base64.StdEncoding.EncodeToString(want.Bytes))
353+
}
356354
}
357355

358356
// TODO(phboneff): this could just be a parseBodyJSONChain test
@@ -413,12 +411,12 @@ func TestAddChainWhitespace(t *testing.T) {
413411
}
414412

415413
log := setupTestLog(t)
416-
server := setupTestServer(t, log, path.Join(prefix, "ct/v1/add-chain"))
414+
server := setupTestServer(t, log, path.Join(prefix, types.AddChainPath))
417415
defer server.Close()
418416

419417
for _, test := range tests {
420418
t.Run(test.descr, func(t *testing.T) {
421-
resp, err := http.Post(server.URL+"/ct/v1/add-chain", "application/json", strings.NewReader(test.body))
419+
resp, err := http.Post(server.URL+types.AddChainPath, "application/json", strings.NewReader(test.body))
422420
if err != nil {
423421
t.Fatalf("http.Post(%s)=(_,%q); want (_,nil)", types.AddChainPath, err)
424422
}
@@ -459,15 +457,15 @@ func TestAddChain(t *testing.T) {
459457
}
460458

461459
log := setupTestLog(t)
462-
server := setupTestServer(t, log, path.Join(prefix, "ct/v1/add-chain"))
460+
server := setupTestServer(t, log, path.Join(prefix, types.AddChainPath))
463461
defer server.Close()
464462

465463
for _, test := range tests {
466464
t.Run(test.descr, func(t *testing.T) {
467465
pool := loadCertsIntoPoolOrDie(t, test.chain)
468466
chain := createJSONChain(t, *pool)
469467

470-
resp, err := http.Post(server.URL+"/ct/v1/add-chain", "application/json", chain)
468+
resp, err := http.Post(server.URL+types.AddChainPath, "application/json", chain)
471469
if err != nil {
472470
t.Fatalf("http.Post(%s)=(_,%q); want (_,nil)", types.AddChainPath, err)
473471
}
@@ -535,15 +533,15 @@ func TestAddPreChain(t *testing.T) {
535533
}
536534

537535
log := setupTestLog(t)
538-
server := setupTestServer(t, log, path.Join(prefix, "ct/v1/add-pre-chain"))
536+
server := setupTestServer(t, log, path.Join(prefix, types.AddPreChainPath))
539537
defer server.Close()
540538

541539
for _, test := range tests {
542540
t.Run(test.descr, func(t *testing.T) {
543541
pool := loadCertsIntoPoolOrDie(t, test.chain)
544542
chain := createJSONChain(t, *pool)
545543

546-
resp, err := http.Post(server.URL+"/ct/v1/add-pre-chain", "application/json", chain)
544+
resp, err := http.Post(server.URL+types.AddPreChainPath, "application/json", chain)
547545
if err != nil {
548546
t.Fatalf("http.Post(%s)=(_,%q); want (_,nil)", types.AddPreChainPath, err)
549547
}

‎internal/testonly/storage/posix/issuers.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
// package posix implements a test issuer storage system on a local filesystem.
16+
// It is not fit for production use.
1517
package posix
1618

1719
import (
@@ -36,15 +38,16 @@ type IssuersStorage string
3638
func NewIssuerStorage(path string) (IssuersStorage, error) {
3739
// Does nothing if the dictory already exists.
3840
if err := os.MkdirAll(path, 0755); err != nil {
39-
return "", fmt.Errorf("failed to create path %q: %w", path, err)
41+
return "", fmt.Errorf("failed to create path %q: %v", path, err)
4042
}
4143
return IssuersStorage(path), nil
4244
}
4345

4446
// keyToObjName converts bytes to filesystem path.
4547
//
4648
// empty keys, and keys including a '/' character are not allowed to avoid
47-
// confusion with directory names.
49+
// confusion with directory names. This list of exclusions is not exhaustive,
50+
// and does not guarantee that it will fit all filesystems.
4851
func (s IssuersStorage) keyToObjName(key []byte) (string, error) {
4952
if string(key) == "" {
5053
return "", fmt.Errorf("key cannot be empty")
@@ -66,15 +69,14 @@ func (s IssuersStorage) AddIssuersIfNotExist(_ context.Context, kv []storage.KV)
6669
if f, err := os.ReadFile(objName); err != nil {
6770
if errors.Is(err, os.ErrNotExist) {
6871
if err := os.WriteFile(objName, kv.V, 0644); err != nil {
69-
return fmt.Errorf("failed to write object %q: %w", objName, err)
72+
return fmt.Errorf("failed to write object %q: %v", objName, err)
7073
}
7174
klog.V(2).Infof("AddIssuersIfNotExist: added %q", objName)
7275
continue
73-
} else {
74-
return fmt.Errorf("failed to read object %q: %w", objName, err)
7576
}
77+
return fmt.Errorf("failed to read object %q: %v", objName, err)
7678
} else if bytes.Equal(f, kv.V) {
77-
klog.V(2).Infof("AddIssuersIfNotExist: object %q already exists, continuing", objName)
79+
klog.V(2).Infof("AddIssuersIfNotExist: object %q already exists with identical contents, continuing", objName)
7880
continue
7981
}
8082
return fmt.Errorf("object %q already exists with different content", objName)

0 commit comments

Comments
 (0)