Skip to content

Commit

Permalink
fix: remove spec hashing as we do not use it (#99)
Browse files Browse the repository at this point in the history
* fix: remove spec hashing as we do not use it

* using ocm with string based pub key value
  • Loading branch information
Skarlso authored Dec 7, 2023
1 parent 14361ac commit a64c598
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 38 deletions.
3 changes: 0 additions & 3 deletions api/v1alpha1/componentsubscription_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,6 @@ type ComponentSubscriptionStatus struct {
// +optional
Signature []v1alpha1.Signature `json:"signature,omitempty"`

// Digest contains the digest of the subscription's spec.
Digest uint64 `json:"specDigest,omitempty"`

// +optional
// +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description=""
// +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].message",description=""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ spec:
value:
description: Value defines a PEM/base64 encoded public key
value.
format: byte
type: string
type: object
required:
Expand Down Expand Up @@ -263,18 +262,13 @@ spec:
value:
description: Value defines a PEM/base64 encoded public key
value.
format: byte
type: string
type: object
required:
- name
- publicKey
type: object
type: array
specDigest:
description: Digest contains the digest of the subscription's spec.
format: int64
type: integer
type: object
type: object
served: true
Expand Down
10 changes: 2 additions & 8 deletions controllers/componentsubscription_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package controllers

import (
"context"
"encoding/base64"
"errors"
"fmt"
"time"
Expand All @@ -14,7 +15,6 @@ import (
"github.com/fluxcd/pkg/apis/meta"
"github.com/fluxcd/pkg/runtime/patch"
rreconcile "github.com/fluxcd/pkg/runtime/reconcile"
"github.com/mitchellh/hashstructure/v2"
ocmv1alpha1 "github.com/open-component-model/ocm-controller/api/v1alpha1"
"github.com/open-component-model/ocm-controller/pkg/status"
ocm2 "github.com/open-component-model/ocm/pkg/contexts/ocm"
Expand Down Expand Up @@ -323,17 +323,11 @@ func (r *ComponentSubscriptionReconciler) signMpasComponent(
{
Name: v1alpha1.InternalSignatureName,
PublicKey: ocmv1alpha1.PublicKey{
Value: pub,
Value: base64.StdEncoding.EncodeToString(pub),
},
},
}

hash, err := hashstructure.Hash(obj.Spec, hashstructure.FormatV2, nil)
if err != nil {
return fmt.Errorf("failed to hash subscription spec: %w", err)
}
obj.Status.Digest = hash

return nil
}

Expand Down
20 changes: 13 additions & 7 deletions controllers/componentsubscription_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@ import (

"github.com/fluxcd/pkg/apis/meta"
"github.com/fluxcd/pkg/runtime/conditions"
"github.com/open-component-model/ocm/pkg/contexts/ocm"
ocmdesc "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc"
v1 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/meta/v1"
"github.com/open-component-model/replication-controller/pkg/sign"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"

"github.com/open-component-model/ocm/pkg/contexts/ocm"
ocmdesc "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc"
v1 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/meta/v1"

"github.com/open-component-model/replication-controller/api/v1alpha1"
"github.com/open-component-model/replication-controller/pkg/ocm/fakes"
)
Expand Down Expand Up @@ -72,7 +72,7 @@ func TestComponentSubscriptionReconciler(t *testing.T) {
},
},
{
name: "mpas enabled component is signed and hashed",
name: "mpas enabled component is signed",
subscription: func() *v1alpha1.ComponentSubscription {
cv := DefaultComponentSubscription.DeepCopy()
return cv
Expand Down Expand Up @@ -101,8 +101,11 @@ func TestComponentSubscriptionReconciler(t *testing.T) {
},
},
}
_, pub, err := sign.GenerateSigningKeyPEMPair()
require.NoError(t, err)
fakeOcm.GetComponentVersionReturnsForName(root.descriptor.ComponentSpec.Name, root, nil)
fakeOcm.GetLatestComponentVersionReturns("v0.0.1", nil)
fakeOcm.SignDestinationComponentReturns(pub, nil)
},
verifyMock: func(fetcher *fakes.MockFetcher) bool {
args := fetcher.SignDestinationComponentCallingArgumentsOnCall(0)
Expand Down Expand Up @@ -265,7 +268,11 @@ func TestComponentSubscriptionReconciler(t *testing.T) {
}

if tt.mpasEnabled {
assert.Equal(t, uint64(0xcc92805632da5940), cv.Status.Digest)
assert.NotEmpty(t, cv.Status.Signature)
sigName := cv.Status.Signature[0].Name
sigKey := cv.Status.Signature[0].PublicKey.Value
assert.Equal(t, v1alpha1.InternalSignatureName, sigName)
assert.NotEmpty(t, sigKey)
}
} else {
assert.EqualError(t, err, tt.err)
Expand All @@ -274,7 +281,6 @@ func TestComponentSubscriptionReconciler(t *testing.T) {
assert.True(t, tt.verifyMock(fakeOcm))
})
}

}

type mockComponent struct {
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@ require (
github.com/fluxcd/pkg/apis/meta v1.1.2
github.com/fluxcd/pkg/runtime v0.42.0
github.com/go-logr/logr v1.3.0
github.com/mitchellh/hashstructure/v2 v2.0.2
github.com/open-component-model/ocm v0.4.0
github.com/open-component-model/ocm-controller v0.18.0
github.com/open-component-model/ocm-controller v0.18.1
github.com/stretchr/testify v1.8.4
k8s.io/api v0.28.1
k8s.io/apimachinery v0.28.1
Expand Down Expand Up @@ -199,6 +198,7 @@ require (
github.com/mitchellh/copystructure v1.2.0 // indirect
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/mitchellh/hashstructure v1.1.0 // indirect
github.com/mitchellh/hashstructure/v2 v2.0.2 // indirect
github.com/mitchellh/mapstructure v1.5.0 // indirect
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/moby/locker v1.0.1 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1180,8 +1180,8 @@ github.com/onsi/gomega v1.30.0 h1:hvMK7xYz4D3HapigLTeGdId/NcfQx1VHMJc60ew99+8=
github.com/onsi/gomega v1.30.0/go.mod h1:9sxs+SwGrKI0+PWe4Fxa9tFQQBG5xSsSbMXOI8PPpoQ=
github.com/open-component-model/ocm v0.4.0 h1:S+rPJGoDnSvxhBn3QS2HXURxugTjCM4XWEJLZSaH6Ek=
github.com/open-component-model/ocm v0.4.0/go.mod h1:7RAqaUMmA4BlwW5ZEUBm8amWIb1TL9FhNigNXQ6wiu0=
github.com/open-component-model/ocm-controller v0.18.0 h1:xyT3mvs0l43nBuYFsLxJWBqibtu1IX2WB6Aa7KJZnzo=
github.com/open-component-model/ocm-controller v0.18.0/go.mod h1:VRwUK7eo+PJYx/XXFo7NvdjW3TsLLdtA1JXdhZzNMCo=
github.com/open-component-model/ocm-controller v0.18.1 h1:bicmTJbBngt5KB0SszgDDQtS2GVyZX6XynBfAzdrv7c=
github.com/open-component-model/ocm-controller v0.18.1/go.mod h1:VRwUK7eo+PJYx/XXFo7NvdjW3TsLLdtA1JXdhZzNMCo=
github.com/opencontainers/go-digest v0.0.0-20170106003457-a6d0ee40d420/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s=
github.com/opencontainers/go-digest v0.0.0-20180430190053-c9281466c8b2/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s=
github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s=
Expand Down
7 changes: 6 additions & 1 deletion pkg/ocm/fakes/fakes.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
// resources and the mock does not compile.
// I.e.: counterfeiter: https://github.com/maxbrunsfeld/counterfeiter/issues/174
type MockFetcher struct {
signDestinationComponentPubKey []byte
getComponentVersionMap map[string]ocm.ComponentVersionAccess
getComponentVersionErr error
getComponentVersionCalledWith [][]any
Expand All @@ -36,7 +37,7 @@ var _ ocm2.Contract = &MockFetcher{}

func (m *MockFetcher) SignDestinationComponent(_ context.Context, component ocm.ComponentVersionAccess) ([]byte, error) {
m.signDestinationComponentCalledWith = append(m.signDestinationComponentCalledWith, []any{component.GetName()})
return nil, nil
return m.signDestinationComponentPubKey, nil
}
func (m *MockFetcher) SignDestinationComponentNotCalled() bool {
return len(m.signDestinationComponentCalledWith) == 0
Expand All @@ -50,6 +51,10 @@ func (m *MockFetcher) SignDestinationComponentCallingArgumentsOnCall(i int) []an
return m.signDestinationComponentCalledWith[i]
}

func (m *MockFetcher) SignDestinationComponentReturns(pub []byte, t error) {
m.signDestinationComponentPubKey = pub
}

func (m *MockFetcher) CreateAuthenticatedOCMContext(ctx context.Context, obj *v1alpha1.ComponentSubscription) (ocm.Context, error) {
return ocm.New(), nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/ocm/ocm.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func (c *Client) VerifyComponent(ctx context.Context, obj *v1alpha1.ComponentSub
err error
)

if signature.PublicKey.Value != nil {
if signature.PublicKey.Value != "" {
cert, err = signature.PublicKey.DecodePublicValue()
} else {
if signature.PublicKey.SecretRef == nil {
Expand Down
9 changes: 1 addition & 8 deletions pkg/ocm/ocm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package ocm

import (
"bytes"
"context"
"encoding/base64"
"os"
Expand Down Expand Up @@ -483,12 +482,6 @@ func TestClient_SignComponent(t *testing.T) {
pub, err := ocmClient.SignDestinationComponent(context.Background(), c)
assert.NoError(t, err)

var buff []byte
buffer := bytes.NewBuffer(buff)
encoder := base64.NewEncoder(base64.StdEncoding, buffer)
_, err = encoder.Write(pub)
require.NoError(t, err)
require.NoError(t, encoder.Close())
cv := &v1alpha1.ComponentSubscription{
ObjectMeta: metav1.ObjectMeta{
Name: "test-name",
Expand All @@ -503,7 +496,7 @@ func TestClient_SignComponent(t *testing.T) {
{
Name: v1alpha1.InternalSignatureName,
PublicKey: ocmv1alpha1.PublicKey{
Value: buffer.Bytes(),
Value: base64.StdEncoding.EncodeToString(pub),
},
},
},
Expand Down

0 comments on commit a64c598

Please sign in to comment.