Skip to content

Commit 9af4291

Browse files
MichalFupsoMichalFupso
authored
Remove default tier prefix from Policy CRD without annotation (projectcalico#10399) (projectcalico#10418)
1 parent 123b426 commit 9af4291

File tree

7 files changed

+177
-35
lines changed

7 files changed

+177
-35
lines changed

‎libcalico-go/lib/backend/k8s/resources/resources.go‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,8 +327,24 @@ func ConvertK8sResourceToCalicoResource(res Resource) error {
327327
// so that they do not get overwritten.
328328
if meta.Name == "" {
329329
// We store the original v3 Name in our annotation when writing NetworkPolicy and GNP objects. If that's present, use it.
330+
// If the kind is Policy and the annotation is not present, we remove the default. prefix to be compatible with previous Calico versions
330331
// Otherwise, fill in the name from the underlying CRD.
331-
meta.Name = rom.GetName()
332+
switch res.GetObjectKind().GroupVersionKind().Kind {
333+
case apiv3.KindGlobalNetworkPolicy:
334+
policy := res.(*apiv3.GlobalNetworkPolicy)
335+
meta.Name = determinePolicyName(rom.GetName(), policy.Spec.Tier)
336+
case apiv3.KindNetworkPolicy:
337+
policy := res.(*apiv3.NetworkPolicy)
338+
meta.Name = determinePolicyName(rom.GetName(), policy.Spec.Tier)
339+
case apiv3.KindStagedGlobalNetworkPolicy:
340+
policy := res.(*apiv3.StagedGlobalNetworkPolicy)
341+
meta.Name = determinePolicyName(rom.GetName(), policy.Spec.Tier)
342+
case apiv3.KindStagedNetworkPolicy:
343+
policy := res.(*apiv3.StagedNetworkPolicy)
344+
meta.Name = determinePolicyName(rom.GetName(), policy.Spec.Tier)
345+
default:
346+
meta.Name = rom.GetName()
347+
}
332348
}
333349
meta.Namespace = rom.GetNamespace()
334350
meta.ResourceVersion = rom.GetResourceVersion()
@@ -365,3 +381,12 @@ func watchOptionsToK8sListOptions(wo api.WatchOptions) metav1.ListOptions {
365381
AllowWatchBookmarks: wo.AllowWatchBookmarks,
366382
}
367383
}
384+
385+
func determinePolicyName(name, tier string) string {
386+
if tier == "default" || tier == "" {
387+
// It's possible the policy does not contain tier, that means it's in the default Tier it's added later by the API server
388+
return strings.TrimPrefix(name, "default.")
389+
}
390+
391+
return name
392+
}

‎libcalico-go/lib/backend/model/keys.go‎

Lines changed: 33 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -646,61 +646,39 @@ func ParseValue(key Key, rawData []byte) (interface{}, error) {
646646

647647
if valueType == reflect.TypeOf(apiv3.NetworkPolicy{}) {
648648
policy := iface.(*apiv3.NetworkPolicy)
649-
annotations := policy.Annotations
650-
if annotations != nil && annotations[metadataAnnotation] != "" {
651-
policy.Name, policy.Annotations, err = parseMetadataAnnotation(annotations)
652-
if err != nil {
653-
return nil, err
654-
}
649+
policy.Name, policy.Annotations, err = determinePolicyName(policy.Name, policy.Spec.Tier, policy.Annotations)
650+
if err != nil {
651+
return nil, err
655652
}
656653
}
657654

658655
if valueType == reflect.TypeOf(apiv3.GlobalNetworkPolicy{}) {
659656
policy := iface.(*apiv3.GlobalNetworkPolicy)
660-
annotations := policy.Annotations
661-
if annotations != nil && annotations[metadataAnnotation] != "" {
662-
policy.Name, policy.Annotations, err = parseMetadataAnnotation(annotations)
663-
if err != nil {
664-
return nil, err
665-
}
657+
policy.Name, policy.Annotations, err = determinePolicyName(policy.Name, policy.Spec.Tier, policy.Annotations)
658+
if err != nil {
659+
return nil, err
666660
}
667661
}
668662

669663
if valueType == reflect.TypeOf(apiv3.StagedNetworkPolicy{}) {
670664
policy := iface.(*apiv3.StagedNetworkPolicy)
671-
annotations := policy.Annotations
672-
if annotations != nil && annotations[metadataAnnotation] != "" {
673-
policy.Name, policy.Annotations, err = parseMetadataAnnotation(annotations)
674-
if err != nil {
675-
return nil, err
676-
}
665+
policy.Name, policy.Annotations, err = determinePolicyName(policy.Name, policy.Spec.Tier, policy.Annotations)
666+
if err != nil {
667+
return nil, err
677668
}
678669
}
679670

680671
if valueType == reflect.TypeOf(apiv3.StagedGlobalNetworkPolicy{}) {
681672
policy := iface.(*apiv3.StagedGlobalNetworkPolicy)
682-
annotations := policy.Annotations
683-
if annotations != nil && annotations[metadataAnnotation] != "" {
684-
policy.Name, policy.Annotations, err = parseMetadataAnnotation(annotations)
685-
if err != nil {
686-
return nil, err
687-
}
673+
policy.Name, policy.Annotations, err = determinePolicyName(policy.Name, policy.Spec.Tier, policy.Annotations)
674+
if err != nil {
675+
return nil, err
688676
}
689677
}
690678

691679
return iface, nil
692680
}
693681

694-
func parseMetadataAnnotation(annotations map[string]string) (string, map[string]string, error) {
695-
meta := &metav1.ObjectMeta{}
696-
err := json.Unmarshal([]byte(annotations[metadataAnnotation]), meta)
697-
if err != nil {
698-
return "", nil, err
699-
}
700-
delete(annotations, metadataAnnotation)
701-
return meta.Name, annotations, nil
702-
}
703-
704682
// SerializeValue serializes a value in the model to a []byte to be stored in the datastore. This
705683
// performs the opposite processing to ParseValue()
706684
func SerializeValue(d *KVPair) ([]byte, error) {
@@ -722,3 +700,24 @@ func SerializeValue(d *KVPair) ([]byte, error) {
722700
}
723701
return json.Marshal(d.Value)
724702
}
703+
704+
// determinePolicyName updates Policy name based on either the projectcalico.org/metadata annotation that was added in 3.30,
705+
// or defaults the name to be returned without the default prefix if no annotation was found. This was the default behaviour in =<3.28
706+
func determinePolicyName(name, tier string, annotations map[string]string) (string, map[string]string, error) {
707+
if annotations != nil && annotations[metadataAnnotation] != "" {
708+
meta := &metav1.ObjectMeta{}
709+
err := json.Unmarshal([]byte(annotations[metadataAnnotation]), meta)
710+
if err != nil {
711+
return "", nil, err
712+
}
713+
delete(annotations, metadataAnnotation)
714+
return meta.Name, annotations, nil
715+
}
716+
717+
if tier == "default" || tier == "" {
718+
// It's possible the policy does not contain tier, that means it's in the default Tier it's added later by the API server
719+
return strings.TrimPrefix(name, "default."), annotations, nil
720+
}
721+
722+
return name, annotations, nil
723+
}

‎libcalico-go/lib/clientv3/globalnetworkpolicy_e2e_test.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package clientv3_test
1616

1717
import (
1818
"context"
19+
"os/exec"
1920
"time"
2021

2122
. "github.com/onsi/ginkgo"
@@ -404,6 +405,19 @@ var _ = testutils.E2eDatastoreDescribe("GlobalNetworkPolicy tests", testutils.Da
404405
Entry("GlobalNetworkPolicy with default tier prefix", "default.netpol", "netpol"),
405406
)
406407

408+
Describe("GlobalNetworkPolicy without name on the projectcalico.org annotation", func() {
409+
It("Should return the name without default prefix", func() {
410+
if config.Spec.DatastoreType == apiconfig.Kubernetes {
411+
// We create the policies as a CRD to prevent the api server adding the correct annotation
412+
err := exec.Command("kubectl", "create", "-f", "../../test/mock-policies.yaml").Run()
413+
Expect(err).ToNot(HaveOccurred())
414+
415+
_, err = c.NetworkPolicies().Get(ctx, "default", "prefix-test-policy", options.GetOptions{})
416+
Expect(err).ToNot(HaveOccurred())
417+
}
418+
})
419+
})
420+
407421
DescribeTable("GlobalNetworkPolicy name validation tests",
408422
func(policyName string, tier string, expectError bool) {
409423
if tier != "default" {

‎libcalico-go/lib/clientv3/networkpolicy_e2e_test.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package clientv3_test
1616

1717
import (
1818
"context"
19+
"os/exec"
1920
"time"
2021

2122
. "github.com/onsi/ginkgo"
@@ -427,6 +428,19 @@ var _ = testutils.E2eDatastoreDescribe("NetworkPolicy tests", testutils.Datastor
427428
Entry("NetworkPolicy with default tier prefix", "default.netpol", "netpol"),
428429
)
429430

431+
Describe("NetworkPolicy without name on the projectcalico.org annotation", func() {
432+
It("Should return the name without default prefix", func() {
433+
if config.Spec.DatastoreType == apiconfig.Kubernetes {
434+
// We create the policies as a CRD to prevent the api server adding the correct annotation
435+
err := exec.Command("kubectl", "create", "-f", "../../test/mock-policies.yaml").Run()
436+
Expect(err).ToNot(HaveOccurred())
437+
438+
_, err = c.NetworkPolicies().Get(ctx, "default", "prefix-test-policy", options.GetOptions{})
439+
Expect(err).ToNot(HaveOccurred())
440+
}
441+
})
442+
})
443+
430444
DescribeTable("NetworkPolicy name validation tests",
431445
func(policyName string, tier string, expectError bool) {
432446
namespace := "default"

‎libcalico-go/lib/clientv3/stagedglobalnetworkpolicy_e2e_test.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package clientv3_test
1616

1717
import (
1818
"context"
19+
"os/exec"
1920
"time"
2021

2122
. "github.com/onsi/ginkgo"
@@ -398,6 +399,19 @@ var _ = testutils.E2eDatastoreDescribe("StagedGlobalNetworkPolicy tests", testut
398399
Entry("StagedGlobalNetworkPolicy with default tier prefix", "default.netpol", "netpol"),
399400
)
400401

402+
Describe("StagedGlobalNetworkPolicy without name on the projectcalico.org annotation", func() {
403+
It("Should return the name without default prefix", func() {
404+
if config.Spec.DatastoreType == apiconfig.Kubernetes {
405+
// We create the policies as a CRD to prevent the api server adding the correct annotation
406+
err := exec.Command("kubectl", "create", "-f", "../../test/mock-policies.yaml").Run()
407+
Expect(err).ToNot(HaveOccurred())
408+
409+
_, err = c.StagedGlobalNetworkPolicies().Get(ctx, "prefix-test-policy", options.GetOptions{})
410+
Expect(err).ToNot(HaveOccurred())
411+
}
412+
})
413+
})
414+
401415
DescribeTable("StagedGlobalNetworkPolicy name validation tests",
402416
func(policyName string, tier string, expectError bool) {
403417
if tier != "default" {

‎libcalico-go/lib/clientv3/stagednetworkpolicy_e2e_test.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package clientv3_test
1616

1717
import (
1818
"context"
19+
"os/exec"
1920
"time"
2021

2122
. "github.com/onsi/ginkgo"
@@ -430,6 +431,19 @@ var _ = testutils.E2eDatastoreDescribe("StagedNetworkPolicy tests", testutils.Da
430431
Entry("StagedNetworkPolicy with default tier prefix", "default.netpol", "netpol"),
431432
)
432433

434+
Describe("StagedNetworkPolicy without name on the projectcalico.org annotation", func() {
435+
It("Should return the name without default prefix", func() {
436+
if config.Spec.DatastoreType == apiconfig.Kubernetes {
437+
// We create the policies as a CRD to prevent the api server adding the correct annotation
438+
err := exec.Command("kubectl", "create", "-f", "../../test/mock-policies.yaml").Run()
439+
Expect(err).ToNot(HaveOccurred())
440+
441+
_, err = c.StagedGlobalNetworkPolicies().Get(ctx, "prefix-test-policy", options.GetOptions{})
442+
Expect(err).ToNot(HaveOccurred())
443+
}
444+
})
445+
})
446+
433447
DescribeTable("StagedNetworkPolicy name validation tests",
434448
func(policyName string, tier string, expectError bool) {
435449
namespace := "default"

‎libcalico-go/test/mock-policies.yaml‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
#These policies are used for testing if the correct policy name is returned when no name is part of the projectcalico.org/metadata annotation
2+
apiVersion: crd.projectcalico.org/v1
3+
kind: NetworkPolicy
4+
metadata:
5+
name: default.prefix-test-policy
6+
namespace: default
7+
annotations:
8+
test: test
9+
projectcalico.org/metadata: '{}'
10+
spec:
11+
tier: default
12+
types:
13+
- Ingress
14+
- Egress
15+
16+
---
17+
18+
apiVersion: crd.projectcalico.org/v1
19+
kind: GlobalNetworkPolicy
20+
metadata:
21+
name: default.prefix-test-policy
22+
namespace: default
23+
annotations:
24+
test: test
25+
projectcalico.org/metadata: '{}'
26+
spec:
27+
tier: default
28+
types:
29+
- Ingress
30+
- Egress
31+
32+
---
33+
34+
apiVersion: crd.projectcalico.org/v1
35+
kind: StagedNetworkPolicy
36+
metadata:
37+
name: default.prefix-test-policy
38+
namespace: default
39+
annotations:
40+
test: test
41+
projectcalico.org/metadata: '{}'
42+
spec:
43+
tier: default
44+
types:
45+
- Ingress
46+
- Egress
47+
48+
---
49+
50+
apiVersion: crd.projectcalico.org/v1
51+
kind: StagedGlobalNetworkPolicy
52+
metadata:
53+
name: default.prefix-test-policy
54+
namespace: default
55+
annotations:
56+
test: test
57+
projectcalico.org/metadata: '{}'
58+
spec:
59+
tier: default
60+
types:
61+
- Ingress
62+
- Egress

0 commit comments

Comments
 (0)