Skip to content

Commit

Permalink
fix: add type meta to cache and references (#4663)
Browse files Browse the repository at this point in the history
Set type meta when saving objects to the store cache and references     
index. This information is otherwise stripped by client-go due to
kubernetes/kubernetes#3030.
  • Loading branch information
randmonkey committed Sep 13, 2023
1 parent f0b4caa commit 663ed21
Show file tree
Hide file tree
Showing 11 changed files with 206 additions and 28 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,18 @@ Adding a new version? You'll need three changes:

### Changes

- Generate a wildcard routes to match all `HTTP` or `GRPC` requests for rules
- Generate wildcard routes to match all `HTTP` or `GRPC` requests for rules
in `HTTPRoute` or `GRPCRoute` if there are no matches in the rule and no
hostnames in their parent objects.
[#4526](https://github.com/Kong/kubernetes-ingress-controller/pull/4528)

### Fixed

- Set type meta of objects when adding them to caches and reference indexers
to ensure that indexes of objects in reference indexers have correct object
kind. This ensures referece relations of objects are stored and indexed
correctly.
[#4663](https://github.com/Kong/kubernetes-ingress-controller/pull/4663)
- Display Service ports on generated Kong services, instead of a static default
value. This change is cosmetic only.
[#4503](https://github.com/Kong/kubernetes-ingress-controller/pull/4503)
Expand Down
7 changes: 7 additions & 0 deletions hack/generators/controllers/networking/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ import (
discoveryv1 "k8s.io/api/discovery/v1"
netv1 "k8s.io/api/networking/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
k8stypes "k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -549,6 +550,12 @@ func (r *{{.PackageAlias}}{{.Kind}}Reconciler) Reconcile(ctx context.Context, re
// get the relevant object
obj := new({{.PackageImportAlias}}.{{.Kind}})
// set type meta to the object
obj.TypeMeta = metav1.TypeMeta{
APIVersion: {{.PackageImportAlias}}.SchemeGroupVersion.String(),
Kind: "{{.Kind}}",
}
if err := r.Get(ctx, req.NamespacedName, obj); err != nil {
if apierrors.IsNotFound(err) {
obj.Namespace = req.Namespace
Expand Down
73 changes: 73 additions & 0 deletions internal/controllers/configuration/zz_generated_controllers.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions internal/controllers/gateway/gateway_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ import (
var (
ErrUnmanagedAnnotation = errors.New("invalid unmanaged annotation value")
gatewayV1beta1Group = gatewayv1beta1.Group(gatewayv1beta1.GroupName)
gatewayTypeMeta = metav1.TypeMeta{
APIVersion: gatewayv1beta1.GroupVersion.String(),
Kind: "Gateway",
}
)

// -----------------------------------------------------------------------------
Expand Down Expand Up @@ -339,6 +343,7 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
// gather the gateway object based on the reconciliation trigger. It's possible for the object
// to be gone at this point in which case it will be ignored.
gateway := new(gatewayv1beta1.Gateway)
gateway.TypeMeta = gatewayTypeMeta
if err := r.Get(ctx, req.NamespacedName, gateway); err != nil {
if apierrors.IsNotFound(err) {
gateway.Namespace = req.Namespace
Expand Down
7 changes: 7 additions & 0 deletions internal/controllers/knative/knative.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/go-logr/logr"
netv1 "k8s.io/api/networking/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
k8stypes "k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -52,6 +53,11 @@ type Knativev1alpha1IngressReconciler struct {
ReferenceIndexers ctrlref.CacheIndexers
}

var knativeIngressTypeMeta = metav1.TypeMeta{
APIVersion: knativev1alpha1.SchemeGroupVersion.String(),
Kind: "Ingress",
}

// SetupWithManager sets up the controller with the Manager.
func (r *Knativev1alpha1IngressReconciler) SetupWithManager(mgr ctrl.Manager) error {
c, err := controller.New("Knativev1alpha1Ingress", mgr, controller.Options{
Expand Down Expand Up @@ -125,6 +131,7 @@ func (r *Knativev1alpha1IngressReconciler) Reconcile(ctx context.Context, req ct

// get the relevant object
obj := new(knativev1alpha1.Ingress)
obj.TypeMeta = knativeIngressTypeMeta
if err := r.Get(ctx, req.NamespacedName, obj); err != nil {
if apierrors.IsNotFound(err) {
obj.Namespace = req.Namespace
Expand Down
27 changes: 26 additions & 1 deletion internal/controllers/reference/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package reference
import (
"fmt"

"github.com/go-logr/logr"
"k8s.io/client-go/tools/cache"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/kong/kubernetes-ingress-controller/v2/internal/controllers"
"github.com/kong/kubernetes-ingress-controller/v2/internal/util"
)

const (
Expand Down Expand Up @@ -36,11 +38,13 @@ func objectKeyFunc(obj client.Object) string {
// provided by cache.Indexer. It could do CRUD on reference records when referrer and referent are
// both provided. It can also list reference records by referrer or by referent.
type CacheIndexers struct {
logger logr.Logger
indexer cache.Indexer
}

func NewCacheIndexers() CacheIndexers {
func NewCacheIndexers(logger logr.Logger) CacheIndexers {
return CacheIndexers{
logger: logger,
indexer: cache.NewIndexer(ObjectReferenceKeyFunc,
cache.Indexers{
IndexNameReferrer: ObjectReferenceIndexerReferrer,
Expand Down Expand Up @@ -88,6 +92,14 @@ func ObjectReferenceIndexerReferent(obj interface{}) ([]string, error) {

// SetObjectReference adds or updates a reference record between referrer and referent in reference cache.
func (c CacheIndexers) SetObjectReference(referrer client.Object, referent client.Object) error {
c.logger.V(util.DebugLevel).Info("Set reference relation",
"referrer_kind", referrer.GetObjectKind().GroupVersionKind().String(),
"referrer_namespace", referrer.GetNamespace(),
"referrer_name", referrer.GetName(),
"referent_kind", referent.GetObjectKind().GroupVersionKind().String(),
"referent_namespace", referent.GetNamespace(),
"referent_name", referent.GetName(),
)
ref := &ObjectReference{
Referrer: referrer,
Referent: referent,
Expand All @@ -97,6 +109,14 @@ func (c CacheIndexers) SetObjectReference(referrer client.Object, referent clien

// DeleteObjectReference deletes the reference record between referrer and referent from reference cache.
func (c CacheIndexers) DeleteObjectReference(referrer client.Object, referent client.Object) error {
c.logger.V(util.DebugLevel).Info("Delete reference relation",
"referrer_kind", referrer.GetObjectKind().GroupVersionKind().String(),
"referrer_namespace", referrer.GetNamespace(),
"referrer_name", referrer.GetName(),
"referent_kind", referent.GetObjectKind().GroupVersionKind().String(),
"referent_namespace", referent.GetNamespace(),
"referent_name", referent.GetName(),
)
ref := &ObjectReference{
Referrer: referrer,
Referent: referent,
Expand Down Expand Up @@ -161,6 +181,11 @@ func (c CacheIndexers) DeleteObjectIfNotReferred(obj client.Object, dataplaneCli
return err
}
if !referred {
c.logger.V(util.DebugLevel).Info("Delete object from cache because it is no longer referred",
"kind", obj.GetObjectKind(),
"namespace", obj.GetNamespace(),
"name", obj.GetName(),
)
return dataplaneClient.DeleteObject(obj)
}
return nil
Expand Down
11 changes: 6 additions & 5 deletions internal/controllers/reference/indexer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"reflect"
"testing"

"github.com/go-logr/logr"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -84,7 +85,7 @@ func TestSetObjectReference(t *testing.T) {
for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
c := NewCacheIndexers()
c := NewCacheIndexers(logr.Discard())
err := c.SetObjectReference(tc.addReferrer, tc.addReferent)
require.NoError(t, err, "should not return error on setting reference")
item, exists, err := c.indexer.Get(&ObjectReference{Referrer: tc.checkReferrer, Referent: tc.checkReferent})
Expand Down Expand Up @@ -129,7 +130,7 @@ func TestDeleteObjectReference(t *testing.T) {
for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
c := NewCacheIndexers()
c := NewCacheIndexers(logr.Discard())
err := c.SetObjectReference(testRefService1, testRefSecret1)
require.NoError(t, err, "should not return error on setting reference")
err = c.SetObjectReference(testRefService1, testRefSecret2)
Expand Down Expand Up @@ -171,7 +172,7 @@ func TestObjectReferred(t *testing.T) {
for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
c := NewCacheIndexers()
c := NewCacheIndexers(logr.Discard())
err := c.SetObjectReference(tc.addReferrer, tc.addReferent)
require.NoError(t, err, "should not return error on setting reference")

Expand Down Expand Up @@ -209,7 +210,7 @@ func TestListReferredObjects(t *testing.T) {
for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
c := NewCacheIndexers()
c := NewCacheIndexers(logr.Discard())
err := c.SetObjectReference(tc.addReferrer, tc.addReferent)
require.NoError(t, err, "should not return error on setting reference")

Expand Down Expand Up @@ -247,7 +248,7 @@ func TestDeleteReferencesByReferrer(t *testing.T) {
for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
c := NewCacheIndexers()
c := NewCacheIndexers(logr.Discard())
err := c.SetObjectReference(testRefService1, testRefSecret1)
require.NoError(t, err, "should not return error on setting reference")
err = c.SetObjectReference(testRefService2, testRefSecret2)
Expand Down
9 changes: 9 additions & 0 deletions internal/controllers/reference/reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ func UpdateReferencesToSecret(
) error {
for nsName := range referencedSecretNameMap {
secret := &corev1.Secret{
TypeMeta: metav1.TypeMeta{
APIVersion: VersionV1,
Kind: KindSecret,
},
ObjectMeta: metav1.ObjectMeta{
Namespace: nsName.Namespace,
Name: nsName.Name,
Expand Down Expand Up @@ -117,6 +121,11 @@ func removeOutdatedReferencesToSecret(
func DeleteReferencesByReferrer(indexers CacheIndexers, dataplaneClient controllers.DataPlaneClient, referrer client.Object) error {
referents, err := indexers.ListReferredObjects(referrer)
if err != nil {
indexers.logger.Error(err, "failed to list referred objects",
"referrer_kind", referrer.GetObjectKind().GroupVersionKind().String(),
"referrer_namespace", referrer.GetNamespace(),
"referrer_name", referrer.GetName(),
)
return err
}

Expand Down
Loading

0 comments on commit 663ed21

Please sign in to comment.