Skip to content

Commit 168f533

Browse files
authored
[release-v1.21] sinkbinding and trustbundle backports (#1810)
* make sinkbinding oidc-token volume mount readOnly (knative#8894) * consistent order for the combined trust bundle (knative#8899) consistent order for combined trust bundle
1 parent 6a52ea6 commit 168f533

File tree

4 files changed

+77
-28
lines changed

4 files changed

+77
-28
lines changed

pkg/apis/sources/v1/sinkbinding_lifecycle.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,12 +262,14 @@ func (sb *SinkBinding) Do(ctx context.Context, ps *duckv1.WithPod) {
262262
ps.Spec.Template.Spec.Containers[i].VolumeMounts = append(ps.Spec.Template.Spec.Containers[i].VolumeMounts, corev1.VolumeMount{
263263
Name: oidcTokenVolumeName,
264264
MountPath: "/oidc",
265+
ReadOnly: true,
265266
})
266267
}
267268
for i := range ps.Spec.Template.Spec.InitContainers {
268269
ps.Spec.Template.Spec.InitContainers[i].VolumeMounts = append(ps.Spec.Template.Spec.InitContainers[i].VolumeMounts, corev1.VolumeMount{
269270
Name: oidcTokenVolumeName,
270271
MountPath: "/oidc",
272+
ReadOnly: true,
271273
})
272274
}
273275
}

pkg/apis/sources/v1/sinkbinding_lifecycle_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -874,6 +874,7 @@ func TestSinkBindingDo(t *testing.T) {
874874
VolumeMounts: []corev1.VolumeMount{{
875875
Name: oidcTokenVolumeName,
876876
MountPath: "/oidc",
877+
ReadOnly: true,
877878
}},
878879
}},
879880
Containers: []corev1.Container{{
@@ -892,6 +893,7 @@ func TestSinkBindingDo(t *testing.T) {
892893
VolumeMounts: []corev1.VolumeMount{{
893894
Name: oidcTokenVolumeName,
894895
MountPath: "/oidc",
896+
ReadOnly: true,
895897
}},
896898
}},
897899
Volumes: []corev1.Volume{{

pkg/eventingtls/trust_bundle.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,8 @@ func PropagateTrustBundles(ctx context.Context, k8s kubernetes.Interface, trustB
190190

191191
if !equality.Semantic.DeepDerivative(expected.Data, p.userCm.Data) ||
192192
!equality.Semantic.DeepDerivative(expected.BinaryData, p.userCm.BinaryData) ||
193-
!equality.Semantic.DeepDerivative(expected.Labels, p.userCm.Labels) {
193+
!equality.Semantic.DeepDerivative(expected.Labels, p.userCm.Labels) ||
194+
!equality.Semantic.DeepDerivative(expected.OwnerReferences, p.userCm.OwnerReferences) {
194195
if err := updateConfigMap(ctx, k8s, expected); err != nil {
195196
return nil, err
196197
}
@@ -309,7 +310,14 @@ func AddTrustBundleVolumesFromConfigMaps(cms []*corev1.ConfigMap, pt *corev1.Pod
309310
}
310311

311312
func combineValidTrustBundles(configMaps []*corev1.ConfigMap, combinedBundle *bytes.Buffer) error {
312-
for _, cm := range configMaps {
313+
// Create a sorted copy of configMaps to ensure consistent ordering without modifying the input slice
314+
sortedCMs := make([]*corev1.ConfigMap, len(configMaps))
315+
copy(sortedCMs, configMaps)
316+
sort.SliceStable(sortedCMs, func(i, j int) bool {
317+
return sortedCMs[i].Name < sortedCMs[j].Name
318+
})
319+
320+
for _, cm := range sortedCMs {
313321

314322
// Ensure the combined bundle is always composed in the same order.
315323
keys := make([]string, 0, len(cm.Data))

pkg/eventingtls/trust_bundle_test.go

Lines changed: 63 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,35 @@ import (
2424
"crypto/x509/pkix"
2525
"encoding/pem"
2626
"math/big"
27-
"strings"
2827
"testing"
2928
"time"
3029

3130
corev1 "k8s.io/api/core/v1"
3231
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3332
)
3433

34+
// parseCertsFromPEM parses all certificates from PEM-encoded data
35+
func parseCertsFromPEM(t *testing.T, pemData []byte) []*x509.Certificate {
36+
var certs []*x509.Certificate
37+
remaining := pemData
38+
for len(remaining) > 0 {
39+
var block *pem.Block
40+
block, remaining = pem.Decode(remaining)
41+
if block == nil {
42+
break
43+
}
44+
if block.Type != "CERTIFICATE" {
45+
continue
46+
}
47+
cert, err := x509.ParseCertificate(block.Bytes)
48+
if err != nil {
49+
t.Fatalf("Failed to parse certificate: %v", err)
50+
}
51+
certs = append(certs, cert)
52+
}
53+
return certs
54+
}
55+
3556
func TestCombineValidTrustBundles(t *testing.T) {
3657
// Helper function to create a valid test certificate
3758
generateValidCert := func() []byte {
@@ -145,6 +166,31 @@ MIIDIzCC corrupted certificate data
145166
expected: string(validCert1) + string(validCert2),
146167
expectError: false,
147168
},
169+
{
170+
name: "Multiple ConfigMaps in reversed order produces consistent output",
171+
configMaps: []*corev1.ConfigMap{
172+
{
173+
ObjectMeta: metav1.ObjectMeta{
174+
Name: "cm2",
175+
Namespace: "test",
176+
},
177+
Data: map[string]string{
178+
"cert2.pem": string(validCert2),
179+
},
180+
},
181+
{
182+
ObjectMeta: metav1.ObjectMeta{
183+
Name: "cm1",
184+
Namespace: "test",
185+
},
186+
Data: map[string]string{
187+
"cert1.pem": string(validCert1),
188+
},
189+
},
190+
},
191+
expected: string(validCert1) + string(validCert2),
192+
expectError: false,
193+
},
148194
{
149195
name: "Multiple certificates in same key",
150196
configMaps: []*corev1.ConfigMap{
@@ -179,7 +225,8 @@ MIIDIzCC corrupted certificate data
179225
},
180226
},
181227
},
182-
expected: string(validCert1) + string(validCert2) + string(validCert3),
228+
// We expect the certs in the alphabetical order of the keys
229+
expected: string(validCert3) + string(validCert1) + string(validCert2),
183230
expectError: false,
184231
},
185232
{
@@ -217,36 +264,26 @@ MIIDIzCC corrupted certificate data
217264
t.Errorf("Did not expect error but got: %v", err)
218265
}
219266

220-
// For valid certs, we need to compare the actual content
221-
// by counting the number of certificates
267+
// For valid certs, verify the actual content and order
222268
if !tt.expectError {
223-
result := buf.String()
269+
result := buf.Bytes()
270+
expected := []byte(tt.expected)
224271

225-
expectedCertCount := strings.Count(tt.expected, "BEGIN CERTIFICATE")
226-
resultCertCount := strings.Count(result, "BEGIN CERTIFICATE")
272+
// Parse expected certificates
273+
expectedCerts := parseCertsFromPEM(t, expected)
274+
// Parse result certificates
275+
resultCerts := parseCertsFromPEM(t, result)
227276

228-
if expectedCertCount != resultCertCount {
277+
// Check count
278+
if len(expectedCerts) != len(resultCerts) {
229279
t.Errorf("Expected %d certificates, got %d",
230-
expectedCertCount, resultCertCount)
280+
len(expectedCerts), len(resultCerts))
231281
}
232282

233-
// Verify the actual certificates are there
234-
// We can't compare the exact string due to PEM encoding variations
235-
// so we count and check if each cert was included
236-
remainingPEM := []byte(result)
237-
for i := 0; i < expectedCertCount; i++ {
238-
var block *pem.Block
239-
block, remainingPEM = pem.Decode(remainingPEM)
240-
241-
if block == nil {
242-
t.Errorf("Failed to decode PEM block %d", i)
243-
break
244-
}
245-
246-
// Verify it's a valid certificate
247-
_, err := x509.ParseCertificate(block.Bytes)
248-
if err != nil {
249-
t.Errorf("PEM block %d is not a valid certificate: %v", i, err)
283+
// Check order and equality
284+
for i := 0; i < len(expectedCerts) && i < len(resultCerts); i++ {
285+
if !expectedCerts[i].Equal(resultCerts[i]) {
286+
t.Errorf("Certificate at index %d does not match expected certificate", i)
250287
}
251288
}
252289
}

0 commit comments

Comments
 (0)