Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(kuma-cp): using policy name with "." causes hash to be inserted in the wrong place on the zone #8240

Merged
Merged
4 changes: 4 additions & 0 deletions pkg/kds/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ func AddHashSuffix(r model.Resource) (model.Resource, error) {
}

newObj := r.Descriptor().NewObject()

// When syncing mesh-scoped resources from Global to Zone, the only possible namespace on Global is system namespace.
// We always trim system namespace in RemoveK8sSystemNamespaceSuffixFromPluginOriginatedResourcesMapper,
// that's why r.GetMeta().GetName() never has a namespace suffix, so we can safely call SyncedNameInZone with it.
newObj.SetMeta(util.NewResourceMeta(hash.SyncedNameInZone(r.GetMeta().GetMesh(), r.GetMeta().GetName()), r.GetMeta().GetMesh()))
_ = newObj.SetSpec(r.GetSpec())

Expand Down
28 changes: 6 additions & 22 deletions pkg/kds/context/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,7 @@ var _ = Describe("Context", func() {
name string
expectedName string
isResourcePluginOriginated bool
scope model.ResourceScope
}

genConfig := func(caseCfg config) kuma_cp.Config {
Expand All @@ -470,6 +471,7 @@ var _ = Describe("Context", func() {
descriptor := model.ResourceTypeDescriptor{
IsPluginOriginated: given.isResourcePluginOriginated,
Resource: core_mesh.NewCircuitBreakerResource(),
Scope: given.scope,
}

meta := &test_model.ResourceMeta{Name: given.name}
Expand Down Expand Up @@ -501,6 +503,7 @@ var _ = Describe("Context", func() {
},
name: "foo.custom-namespace",
expectedName: "foo-zxw6c95d42zfz9cc",
scope: model.ScopeMesh,
}),
Entry("shouldn't be removed when store type is kubernetes "+
"and resource isn't plugin originated", testCase{
Expand All @@ -509,28 +512,9 @@ var _ = Describe("Context", func() {
storeType: config_store.KubernetesStore,
k8sSystemNamespace: "custom-namespace",
},
name: "foo.custom-namespace",
expectedName: "foo-zxw6c95d42zfz9cc.custom-namespace",
}),
Entry("shouldn't be removed when store type is not kubernetes",
testCase{
isResourcePluginOriginated: true,
config: config{
storeType: config_store.PostgresStore,
k8sSystemNamespace: "custom-namespace",
},
name: "foo.custom-namespace",
expectedName: "foo-zxw6c95d42zfz9cc.custom-namespace",
}),
Entry("shouldn't be removed when suffix is not k8s system "+
"namespace", testCase{
isResourcePluginOriginated: true,
config: config{
storeType: config_store.KubernetesStore,
k8sSystemNamespace: "kuma-system",
},
name: "foo.custom-namespace",
expectedName: "foo-zxw6c95d42zfz9cc.custom-namespace",
name: "foo.default",
expectedName: "foo.default",
scope: model.ScopeGlobal,
}),
)
})
Expand Down
8 changes: 1 addition & 7 deletions pkg/kds/hash/hash.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,13 @@ import (

"k8s.io/apimachinery/pkg/util/rand"
k8s_strings "k8s.io/utils/strings"

util_k8s "github.com/kumahq/kuma/pkg/util/k8s"
)

// SyncedNameInZone returns the resource's name after syncing from Global to Zone.
// It creates a new name by adding a hash suffix constructed from the 'mesh' and
// the original 'name'.
func SyncedNameInZone(mesh, name string) string {
if n, ns, err := util_k8s.CoreNameToK8sName(name); err == nil {
return util_k8s.K8sNamespacedNameToCoreName(addSuffix(n, hash(mesh, n)), ns)
} else {
return addSuffix(name, hash(mesh, name))
}
return addSuffix(name, hash(mesh, name))
}

func addSuffix(name, hash string) string {
Expand Down
17 changes: 4 additions & 13 deletions pkg/plugins/resources/k8s/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (

"github.com/kumahq/kuma/pkg/core/resources/model"
k8s_model "github.com/kumahq/kuma/pkg/plugins/resources/k8s/native/pkg/model"
util_k8s "github.com/kumahq/kuma/pkg/util/k8s"
)

type ResourceMapperFunc func(resource model.Resource, namespace string) (k8s_model.KubernetesObject, error)
Expand Down Expand Up @@ -33,21 +32,13 @@ func NewInferenceMapper(systemNamespace string, kubeFactory KubeFactory) Resourc
return nil, err
}
if rs.Scope() == k8s_model.ScopeNamespace {
name, ns, err := util_k8s.CoreNameToK8sName(resource.GetMeta().GetName())
if err != nil {
// if the original resource doesn't look like a kubernetes name ("name"."namespace")`, just use the default namespace.
// this is in the case where someone calls this on a universal cluster. Exporting is a use-case for this.
ns = systemNamespace
name = resource.GetMeta().GetName()
}
if namespace != "" { // If the user is forcing the namespace accept it.
ns = namespace
rs.SetNamespace(namespace)
} else {
rs.SetNamespace(systemNamespace)
}
rs.SetName(name)
rs.SetNamespace(ns)
} else {
rs.SetName(resource.GetMeta().GetName())
}
rs.SetName(resource.GetMeta().GetName())
rs.SetMesh(resource.GetMeta().GetMesh())
rs.SetCreationTimestamp(v1.NewTime(resource.GetMeta().GetCreationTime()))
rs.SetSpec(resource.GetSpec())
Expand Down
13 changes: 0 additions & 13 deletions pkg/plugins/resources/k8s/mapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,6 @@ var _ = Describe("KubernetesStore template", func() {
Expect(res.GetNamespace()).To(Equal("core-ns"))
})

It("works with namespace in name on inference", func() {
lobkovilya marked this conversation as resolved.
Show resolved Hide resolved
in := core_mtp.NewMeshTrafficPermissionResource()
in.SetMeta(&rest_v1alpha1.ResourceMeta{Name: "foo.namespace", Mesh: "default"})

mapper := k8s.NewInferenceMapper("core-ns", &k8s.SimpleKubeFactory{KubeTypes: kubeTypes})
res, err := mapper(in, "")

Expect(err).ToNot(HaveOccurred())
Expect(res.GetMesh()).To(Equal("default"))
Expect(res.GetName()).To(Equal("foo"))
Expect(res.GetNamespace()).To(Equal("namespace"))
})

It("works passing namespace on inference", func() {
in := core_mtp.NewMeshTrafficPermissionResource()
in.SetMeta(&rest_v1alpha1.ResourceMeta{Name: "foo", Mesh: "default"})
Expand Down
Loading