Skip to content

Commit

Permalink
chore: clean up comments format and removed unused code (#273)
Browse files Browse the repository at this point in the history
go fmt.
Cleaned up comments format.
Removed unused code in unit tests.

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
  • Loading branch information
Two-Hearts authored Feb 10, 2023
1 parent 1b2ec27 commit 7987c85
Show file tree
Hide file tree
Showing 13 changed files with 76 additions and 56 deletions.
8 changes: 5 additions & 3 deletions config/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ import (
"crypto/tls"
"errors"
"fmt"
"io/fs"

"github.com/notaryproject/notation-go/internal/slices"
"github.com/notaryproject/notation-go/log"
"github.com/notaryproject/notation-go/plugin"
"io/fs"

"github.com/notaryproject/notation-go/dir"
set "github.com/notaryproject/notation-go/internal/container"
Expand Down Expand Up @@ -148,7 +149,7 @@ func (s *SigningKeys) Remove(keyName ...string) ([]string, error) {
}
s.Keys = slices.Delete(s.Keys, idx)
deletedNames = append(deletedNames, name)
if s.Default !=nil && *s.Default == name {
if s.Default != nil && *s.Default == name {
s.Default = nil
}
}
Expand Down Expand Up @@ -202,7 +203,8 @@ func LoadSigningKeys() (*SigningKeys, error) {
return &config, nil
}

// LoadExecSaveSigningKeys loads signing key, executes given function and then saves the signing key
// LoadExecSaveSigningKeys loads signing key, executes given function and
// then saves the signing key
func LoadExecSaveSigningKeys(fn func(keys *SigningKeys) error) error {
// core process
signingKeys, err := LoadSigningKeys()
Expand Down
8 changes: 4 additions & 4 deletions config/keys_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@ import (
"context"
"crypto/x509"
"encoding/pem"
"github.com/notaryproject/notation-core-go/testhelper"
"os"
"path/filepath"
"reflect"
"testing"

"github.com/notaryproject/notation-core-go/testhelper"
"github.com/notaryproject/notation-go/dir"
)

var sampleSigningKeysInfo = SigningKeys{
Default: Ptr[string]("wabbit-networks"),
Default: Ptr("wabbit-networks"),
Keys: []KeySuite{
{
Name: "wabbit-networks",
Expand Down Expand Up @@ -153,14 +153,14 @@ func TestSaveSigningKeys(t *testing.T) {
expectedErr := "malformed signingkeys.json: default key 'missing-default' not found"
dir.UserConfigDir = t.TempDir()
invalidDefaultSignKeysInfo := deepCopySigningKeys(sampleSigningKeysInfo)
invalidDefaultSignKeysInfo.Default = Ptr[string]("missing-default")
invalidDefaultSignKeysInfo.Default = Ptr("missing-default")
err := invalidDefaultSignKeysInfo.Save()
if err == nil || err.Error() != expectedErr {
t.Errorf("Save signingkeys.json failed, error expected = \"%v\" but found = \"%v\"", expectedErr, err)
}

expectedErr = "malformed signingkeys.json: default key name cannot be empty"
invalidDefaultSignKeysInfo.Default = Ptr[string]("")
invalidDefaultSignKeysInfo.Default = Ptr("")
err = invalidDefaultSignKeysInfo.Save()
if err == nil || err.Error() != expectedErr {
t.Errorf("Save signingkeys.json failed, error expected = \"%v\" but found = \"%v\"", expectedErr, err)
Expand Down
18 changes: 12 additions & 6 deletions errors.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package notation

//
// ErrorPushSignatureFailed is used when failed to push signature to the
// target registry.
type ErrorPushSignatureFailed struct {
Msg string
}
Expand All @@ -12,7 +13,8 @@ func (e ErrorPushSignatureFailed) Error() string {
return "failed to push signature to registry"
}

// ErrorVerificationInconclusive is used when signature verification fails due to a runtime error (e.g. a network error)
// ErrorVerificationInconclusive is used when signature verification fails due
// to a runtime error (e.g. a network error)
type ErrorVerificationInconclusive struct {
Msg string
}
Expand All @@ -24,7 +26,8 @@ func (e ErrorVerificationInconclusive) Error() string {
return "signature verification was inclusive due to an unexpected error"
}

// ErrorNoApplicableTrustPolicy is used when there is no trust policy that applies to the given artifact
// ErrorNoApplicableTrustPolicy is used when there is no trust policy that
// applies to the given artifact
type ErrorNoApplicableTrustPolicy struct {
Msg string
}
Expand All @@ -36,7 +39,8 @@ func (e ErrorNoApplicableTrustPolicy) Error() string {
return "there is no applicable trust policy for the given artifact"
}

// ErrorSignatureRetrievalFailed is used when notation is unable to retrieve the digital signature/s for the given artifact
// ErrorSignatureRetrievalFailed is used when notation is unable to retrieve the
// digital signature/s for the given artifact
type ErrorSignatureRetrievalFailed struct {
Msg string
}
Expand All @@ -48,7 +52,8 @@ func (e ErrorSignatureRetrievalFailed) Error() string {
return "unable to retrieve the digital signature from the registry"
}

// ErrorVerificationFailed is used when it is determined that the digital signature/s is not valid for the given artifact
// ErrorVerificationFailed is used when it is determined that the digital
// signature/s is not valid for the given artifact
type ErrorVerificationFailed struct {
Msg string
}
Expand All @@ -60,7 +65,8 @@ func (e ErrorVerificationFailed) Error() string {
return "signature verification failed"
}

// ErrorUserMetadataVerificationFailed is used when the signature does not contain the user specified metadata
// ErrorUserMetadataVerificationFailed is used when the signature does not
// contain the user specified metadata
type ErrorUserMetadataVerificationFailed struct {
Msg string
}
Expand Down
3 changes: 2 additions & 1 deletion internal/pkix/pkix.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ func ParseDistinguishedName(name string) (map[string]string, error) {
return attrKeyValue, nil
}

// IsSubsetDN returns true if dn1 is a subset of dn2 i.e. every key/value pair of dn1 has a matching key/value pair in dn2, otherwise returns false
// IsSubsetDN returns true if dn1 is a subset of dn2 i.e. every key/value pair
// of dn1 has a matching key/value pair in dn2, otherwise returns false
func IsSubsetDN(dn1 map[string]string, dn2 map[string]string) bool {
for key := range dn1 {
if dn1[key] != dn2[key] {
Expand Down
19 changes: 12 additions & 7 deletions notation.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,17 @@ var reservedAnnotationPrefixes = [...]string{"io.cncf.notary"}

// SignOptions contains parameters for Signer.Sign.
type SignOptions struct {
// ArtifactReference sets the reference of the artifact that needs to be signed.
// ArtifactReference sets the reference of the artifact that needs to be
// signed.
ArtifactReference string

// SignatureMediaType is the envelope type of the signature.
// Currently both `application/jose+json` and `application/cose` are
// supported.
SignatureMediaType string

// ExpiryDuration identifies the expiry duration of the resulted signature. Zero value
// represents no expiry duration.
// ExpiryDuration identifies the expiry duration of the resulted signature.
// Zero value represents no expiry duration.
ExpiryDuration time.Duration

// PluginConfig sets or overrides the plugin configuration.
Expand All @@ -51,7 +52,8 @@ type SignOptions struct {
type RemoteSignOptions struct {
SignOptions

// UserMetadata contains key-value pairs that are added to the signature payload
// UserMetadata contains key-value pairs that are added to the signature
// payload
UserMetadata map[string]string
}

Expand All @@ -66,7 +68,8 @@ type Signer interface {

// signerAnnotation facilitates return of manifest annotations by signers
type signerAnnotation interface {
// PluginAnnotations returns signature manifest annotations returned from plugin
// PluginAnnotations returns signature manifest annotations returned from
// plugin
PluginAnnotations() map[string]string
}

Expand Down Expand Up @@ -229,7 +232,8 @@ type VerifyOptions struct {
// PluginConfig is a map of plugin configs.
PluginConfig map[string]string

// UserMetadata contains key-value pairs that must be present in the signature
// UserMetadata contains key-value pairs that must be present in the
// signature
UserMetadata map[string]string
}

Expand Down Expand Up @@ -257,7 +261,8 @@ type RemoteVerifyOptions struct {
// to zero, an error will be returned.
MaxSignatureAttempts int

// UserMetadata contains key-value pairs that must be present in the signature
// UserMetadata contains key-value pairs that must be present in the
// signature
UserMetadata map[string]string
}

Expand Down
10 changes: 0 additions & 10 deletions notation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,16 +270,6 @@ func (s *verifyMetadataSigner) Sign(ctx context.Context, desc ocispec.Descriptor
return []byte("ABC"), &signature.SignerInfo{}, nil
}

type dummyPluginSigner struct{}

func (s *dummyPluginSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts SignOptions) ([]byte, *signature.SignerInfo, error) {
return []byte("ABC"), &signature.SignerInfo{}, nil
}

func (s *dummyPluginSigner) PluginAnnotations() map[string]string {
return map[string]string{"hi": "hola"}
}

type dummyVerifier struct {
TrustPolicyDoc *trustpolicy.Document
PluginManager plugin.Manager
Expand Down
5 changes: 0 additions & 5 deletions plugin/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@ var invalidMetadataName = proto.GetMetadataResponse{
SupportedContractVersions: []string{"1.0"}, Capabilities: []proto.Capability{proto.CapabilitySignatureGenerator},
}

var validMetadataBar = proto.GetMetadataResponse{
Name: "bar", Description: "friendly", Version: "1", URL: "example.com",
SupportedContractVersions: []string{"1"}, Capabilities: []proto.Capability{proto.CapabilitySignatureGenerator},
}

var invalidContractVersionMetadata = proto.GetMetadataResponse{
Name: "foo", Description: "friendly", Version: "1", URL: "example.com",
SupportedContractVersions: []string{"110.0"}, Capabilities: []proto.Capability{proto.CapabilitySignatureGenerator},
Expand Down
3 changes: 2 additions & 1 deletion plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ type SignPlugin interface {
// GenerateSignature generates the raw signature based on the request.
GenerateSignature(ctx context.Context, req *proto.GenerateSignatureRequest) (*proto.GenerateSignatureResponse, error)

// GenerateEnvelope generates the Envelope with signature based on the request.
// GenerateEnvelope generates the Envelope with signature based on the
// request.
GenerateEnvelope(ctx context.Context, req *proto.GenerateEnvelopeRequest) (*proto.GenerateEnvelopeResponse, error)
}

Expand Down
3 changes: 2 additions & 1 deletion plugin/proto/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ package proto

import "time"

// VerifySignatureRequest contains the parameters passed in a verify-signature request.
// VerifySignatureRequest contains the parameters passed in a verify-signature
// request.
type VerifySignatureRequest struct {
ContractVersion string `json:"contractVersion"`
Signature Signature `json:"signature"`
Expand Down
3 changes: 2 additions & 1 deletion signer/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,8 @@ func isPayloadDescriptorValid(originalDesc, newDesc ocispec.Descriptor) bool {

func areUnknownAttributesAdded(content []byte) []string {
var targetArtifactMap map[string]interface{}
// Ignoring error because we already successfully unmarshalled before this point
// Ignoring error because we already successfully unmarshalled before this
// point
_ = json.Unmarshal(content, &targetArtifactMap)
descriptor := targetArtifactMap["targetArtifact"].(map[string]interface{})

Expand Down
22 changes: 15 additions & 7 deletions verifier/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ import (
)

const (
// HeaderVerificationPlugin specifies the name of the verification plugin that should be used to verify the signature.
// HeaderVerificationPlugin specifies the name of the verification plugin
// that should be used to verify the signature.
HeaderVerificationPlugin = "io.cncf.notary.verificationPlugin"

// HeaderVerificationPluginMinVersion specifies the minimum version of the verification plugin that should be used to verify the signature.
// HeaderVerificationPluginMinVersion specifies the minimum version of the
// verification plugin that should be used to verify the signature.
HeaderVerificationPluginMinVersion = "io.cncf.notary.verificationPluginMinVersion"
)

Expand Down Expand Up @@ -72,8 +74,11 @@ func loadX509TrustStores(ctx context.Context, scheme signature.SigningScheme, po
return certificates, nil
}

// isCriticalFailure checks whether a VerificationResult fails the entire signature verification workflow.
// signature verification workflow is considered failed if there is a VerificationResult with "Enforced" as the action but the result was unsuccessful
// isCriticalFailure checks whether a VerificationResult fails the entire
// signature verification workflow.
// signature verification workflow is considered failed if there is a
// VerificationResult with "Enforced" as the action but the result was
// unsuccessful.
func isCriticalFailure(result *notation.ValidationResult) bool {
return result.Action == trustpolicy.ActionEnforce && result.Error != nil
}
Expand All @@ -82,15 +87,18 @@ func getNonPluginExtendedCriticalAttributes(signerInfo *signature.SignerInfo) []
var criticalExtendedAttrs []signature.Attribute
for _, attr := range signerInfo.SignedAttributes.ExtendedAttributes {
attrStrKey, ok := attr.Key.(string)
if ok && !slices.Contains(VerificationPluginHeaders, attrStrKey) { // filter the plugin extended attributes
// TODO support other attribute types (COSE attribute keys can be numbers)
// filter the plugin extended attributes
if ok && !slices.Contains(VerificationPluginHeaders, attrStrKey) {
// TODO support other attribute types
// (COSE attribute keys can be numbers)
criticalExtendedAttrs = append(criticalExtendedAttrs, attr)
}
}
return criticalExtendedAttrs
}

// extractCriticalStringExtendedAttribute extracts a critical string Extended attribute from a signer.
// extractCriticalStringExtendedAttribute extracts a critical string Extended
// attribute from a signer.
func extractCriticalStringExtendedAttribute(signerInfo *signature.SignerInfo, key string) (string, error) {
attr, err := signerInfo.ExtendedAttribute(key)
// not exist
Expand Down
6 changes: 4 additions & 2 deletions verifier/trustpolicy/trustpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -503,9 +503,11 @@ type parsedDN struct {
ParsedMap map[string]string
}

// validateRegistryScopeFormat validates if a scope is following the format defined in distribution spec
// validateRegistryScopeFormat validates if a scope is following the format
// defined in distribution spec
func validateRegistryScopeFormat(scope string) error {
// Domain and Repository regexes are adapted from distribution implementation
// Domain and Repository regexes are adapted from distribution
// implementation
// https://github.com/distribution/distribution/blob/main/reference/regexp.go#L31
domainRegexp := regexp.MustCompile(`^(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])(?:(?:\.(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+)?(?::[0-9]+)?$`)
repositoryRegexp := regexp.MustCompile(`^[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?(?:(?:/[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?)+)?$`)
Expand Down
Loading

0 comments on commit 7987c85

Please sign in to comment.