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.

Sign up for GitHub

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

Drop KongCredential from KIC and configuration/v1 CRDs #862

Merged
merged 5 commits into from
Sep 29, 2020
Merged

Drop KongCredential from KIC and configuration/v1 CRDs #862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0b29a82
chore(parser): delete KongCredential support
mflendrich Sep 18, 2020
7285d37
chore(controller): delete KongCredential support
mflendrich Sep 18, 2020
1043ac2
chore(store): delete KongCredential support
mflendrich Sep 18, 2020
c60e5be
chore(apis): delete KongCredential from v1
mflendrich Sep 18, 2020
f59bf4e
chore(deploy): remove the KongCredential CRD
mflendrich Sep 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions cli/ingress-controller/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,11 +423,6 @@ func main() {
cacheStores.Consumer = kongConsumerInformer.GetStore()
informers = append(informers, kongConsumerInformer)

kongCredentialInformer := kongInformerFactory.Configuration().V1().KongCredentials().Informer()
kongCredentialInformer.AddEventHandler(reh)
cacheStores.Credential = kongCredentialInformer.GetStore()
informers = append(informers, kongCredentialInformer)

if controllerConfig.EnableKnativeIngressSupport {
knativeIngressInformer := knativeInformerFactory.Networking().V1alpha1().Ingresses().Informer()
knativeIngressInformer.AddEventHandler(reh)
Expand Down
38 changes: 0 additions & 38 deletions deploy/manifests/base/custom-types.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,44 +192,6 @@ spec:
subresources:
status: {}

---
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: kongcredentials.configuration.konghq.com
spec:
group: configuration.konghq.com
version: v1
scope: Namespaced
names:
kind: KongCredential
plural: kongcredentials
additionalPrinterColumns:
- name: Credential-type
type: string
description: Type of credential
JSONPath: .type
- name: Age
type: date
description: Age
JSONPath: .metadata.creationTimestamp
- name: Consumer-Ref
type: string
description: Owner of the credential
JSONPath: .consumerRef
validation:
openAPIV3Schema:
required:
- consumerRef
- type
properties:
consumerRef:
type: string
type:
type: string
subresources:
status: {}

---
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
Expand Down
37 changes: 0 additions & 37 deletions deploy/single/all-in-one-dbless-k4k8s-enterprise.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,43 +120,6 @@ spec:
---
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: kongcredentials.configuration.konghq.com
spec:
additionalPrinterColumns:
- JSONPath: .type
description: Type of credential
name: Credential-type
type: string
- JSONPath: .metadata.creationTimestamp
description: Age
name: Age
type: date
- JSONPath: .consumerRef
description: Owner of the credential
name: Consumer-Ref
type: string
group: configuration.konghq.com
names:
kind: KongCredential
plural: kongcredentials
scope: Namespaced
subresources:
status: {}
validation:
openAPIV3Schema:
properties:
consumerRef:
type: string
type:
type: string
required:
- consumerRef
- type
version: v1
---
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: kongingresses.configuration.konghq.com
spec:
Expand Down
37 changes: 0 additions & 37 deletions deploy/single/all-in-one-dbless.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,43 +120,6 @@ spec:
---
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: kongcredentials.configuration.konghq.com
spec:
additionalPrinterColumns:
- JSONPath: .type
description: Type of credential
name: Credential-type
type: string
- JSONPath: .metadata.creationTimestamp
description: Age
name: Age
type: date
- JSONPath: .consumerRef
description: Owner of the credential
name: Consumer-Ref
type: string
group: configuration.konghq.com
names:
kind: KongCredential
plural: kongcredentials
scope: Namespaced
subresources:
status: {}
validation:
openAPIV3Schema:
properties:
consumerRef:
type: string
type:
type: string
required:
- consumerRef
- type
version: v1
---
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: kongingresses.configuration.konghq.com
spec:
Expand Down
37 changes: 0 additions & 37 deletions deploy/single/all-in-one-postgres-enterprise.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,43 +120,6 @@ spec:
---
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: kongcredentials.configuration.konghq.com
spec:
additionalPrinterColumns:
- JSONPath: .type
description: Type of credential
name: Credential-type
type: string
- JSONPath: .metadata.creationTimestamp
description: Age
name: Age
type: date
- JSONPath: .consumerRef
description: Owner of the credential
name: Consumer-Ref
type: string
group: configuration.konghq.com
names:
kind: KongCredential
plural: kongcredentials
scope: Namespaced
subresources:
status: {}
validation:
openAPIV3Schema:
properties:
consumerRef:
type: string
type:
type: string
required:
- consumerRef
- type
version: v1
---
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: kongingresses.configuration.konghq.com
spec:
Expand Down
37 changes: 0 additions & 37 deletions deploy/single/all-in-one-postgres.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,43 +120,6 @@ spec:
---
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: kongcredentials.configuration.konghq.com
spec:
additionalPrinterColumns:
- JSONPath: .type
description: Type of credential
name: Credential-type
type: string
- JSONPath: .metadata.creationTimestamp
description: Age
name: Age
type: date
- JSONPath: .consumerRef
description: Owner of the credential
name: Consumer-Ref
type: string
group: configuration.konghq.com
names:
kind: KongCredential
plural: kongcredentials
scope: Namespaced
subresources:
status: {}
validation:
openAPIV3Schema:
properties:
consumerRef:
type: string
type:
type: string
required:
- consumerRef
- type
version: v1
---
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: kongingresses.configuration.konghq.com
spec:
Expand Down
86 changes: 0 additions & 86 deletions internal/ingress/controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ import (
"github.com/kong/kubernetes-ingress-controller/internal/ingress/store"
"github.com/kong/kubernetes-ingress-controller/internal/ingress/task"
"github.com/kong/kubernetes-ingress-controller/internal/ingress/utils"
configurationv1 "github.com/kong/kubernetes-ingress-controller/pkg/apis/configuration/v1"
configClientSet "github.com/kong/kubernetes-ingress-controller/pkg/client/configuration/clientset/versioned"
"github.com/mitchellh/mapstructure"
"github.com/sirupsen/logrus"
"golang.org/x/sync/errgroup"
networking "k8s.io/api/networking/v1beta1"
Expand Down Expand Up @@ -279,14 +277,6 @@ func (n *KongController) Start() {
if evt, ok := event.(Event); ok {
n.Logger.WithField("event_type", evt.Type).Debugf("event received")
n.syncQueue.Enqueue(evt.Obj)
// TODO retry for ephermal error conditions
// This function is called outside the task queue because event
// information is currently shielded from the sync function.
// Sync function syncs everything, no matter what the event is
err := n.handleBasicAuthUpdates(ctx, evt)
if err != nil {
n.Logger.Errorf("failed to update basic-auth credentials: %v", err)
}
} else {
n.Logger.WithField("event_type", evt.Type).Errorf("invalid event received")
}
Expand Down Expand Up @@ -323,79 +313,3 @@ func (n *KongController) Stop() error {

return nil
}

// handleBasicAuthUpdates updates basic-auth password field in Kong whenever it is changed.
//
// Kong hashes basic-auth passwords in DB and API responses once created.
// Due to this reason, one can't perform a 'diff' with them.
// This function filters for basic-auth password changes and applies them
// to Kong as they happen.
func (n *KongController) handleBasicAuthUpdates(ctx context.Context, event Event) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it seems like we introduced a bug when we added in the Secret-based credentials.
This function should be triggered when basic-auth creds in a secret change.
The bug has not come up because it applies to basic-auth credentials and DB-mode of Kong, which is not a common situation (I wonder where the users are who prompted me to add this function, we had quite a few reports for this problem in the past).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracking this here: #865

Not a blocker for this PR.

if !n.elector.IsLeader() {
return nil
}
if n.cfg.Kong.InMemory {
return nil
}
if event.Type != UpdateEvent {
return nil
}
newCred, ok := event.Obj.(*configurationv1.KongCredential)
if !ok {
return nil
}
if newCred.ConsumerRef == "" {
return nil
}
oldCred, ok := event.Old.(*configurationv1.KongCredential)
if !ok {
return nil
}
// if the credential type was changed, then the basic-auth
// credential will be either created or deleted by decK
if oldCred.Type != "basic-auth" && newCred.Type != "basic-auth" {
return nil
}
oldPassword := oldCred.Config["password"]
newPassword := newCred.Config["password"]
if oldPassword == newPassword {
return nil
}
// there was an update to a basic-auth credential and the password
// has changed, sync it
var cred kong.BasicAuth
decoder, err := mapstructure.NewDecoder(
&mapstructure.DecoderConfig{TagName: "json",
Result: &cred,
})
if err != nil {
return fmt.Errorf("failed to create a decoder: %w", err)
}
err = decoder.Decode(newCred.Config)
if err != nil {
return fmt.Errorf("error decoding credential '%v/%v': %w",
newCred.Namespace, newCred.Name, err)
}

kongConsumer, err := n.store.GetKongConsumer(newCred.Namespace,
newCred.ConsumerRef)
if err != nil {
return fmt.Errorf("error searching for consumer '%v/%v': %w",
newCred.Namespace, newCred.ConsumerRef, err)
}
username := kongConsumer.Username
client := n.cfg.Kong.Client

// find the ID of the cred from Kong
outdatedCred, err := client.BasicAuths.Get(ctx, &username, cred.Username)
if err != nil {
return fmt.Errorf("fetching basic-auth credential: %w", err)
}
cred.ID = outdatedCred.ID
// update it
_, err = client.BasicAuths.Create(ctx, &username, &cred)
if err != nil {
return fmt.Errorf("updating basic-auth credential: %w", err)
}
return nil
}
38 changes: 0 additions & 38 deletions internal/ingress/controller/parser/kongstate/kongstate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,44 +86,6 @@ func (ks *KongState) FillConsumersAndCredentials(log logrus.FieldLogger, s store
consumerIndex[kConsumer.Namespace+"/"+kConsumer.Name] = c
}

// legacy attach credentials
credentials := s.ListKongCredentials()
if len(credentials) > 0 {
log.Warnf("deprecated KongCredential resource in use; " +
"please use secret-based credentials, " +
"KongCredential resource will be removed in future")
}
for _, credential := range credentials {
log = log.WithFields(logrus.Fields{
"kongcredential_name": credential.Name,
"kongcredential_namespace": credential.Namespace,
"consumerRef": credential.ConsumerRef,
})
cons, ok := consumerIndex[credential.Namespace+"/"+
credential.ConsumerRef]
if !ok {
continue
}
if credential.Type == "" {
log.Errorf("invalid KongCredential: no Type provided")
continue
}
if !supportedCreds.Has(credential.Type) {
log.Errorf("invalid KongCredential: invalid Type provided")
continue
}
if credential.Config == nil {
log.Errorf("invalid KongCredential: empty config")
continue
}
err := cons.SetCredential(log, credential.Type, credential.Config)
if err != nil {
log.Errorf("failed to provision credential: %v", err)
continue
}
consumerIndex[credential.Namespace+"/"+credential.ConsumerRef] = cons
}

// populate the consumer in the state
for _, c := range consumerIndex {
ks.Consumers = append(ks.Consumers, c)
Expand Down
Loading