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

Change kongCredType to a label #2502

Closed
1 of 4 tasks
rainest opened this issue May 17, 2022 · 4 comments · Fixed by #4825
Closed
1 of 4 tasks

Change kongCredType to a label #2502

rainest opened this issue May 17, 2022 · 4 comments · Fixed by #4825

Comments

@rainest
Copy link
Contributor

rainest commented May 17, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Problem Statement

Currently the controller uses a special kongCredType key in Secret data to identify whether a Secret contains Kong credentials and what type of credential the Secret contains.

Admission webhooks can only filter resources based on labels. We have no means of filtering out only the Secrets we need because we have no label for them.

Proposed Solution

Add controller support for determining which Secrets are relevant and determining their type using a label. Optionally remove the code for doing so using the kongCredType key. Update the admission webhook definition to include an objectSelector that excludes Secrets without the label.

Additional information

Broken out of #2431.

Removing support for the kongCredType key is a breaking change. Users will need to update existing resources to use labels instead. We can ease this process by:

  • Not immediately dropping support for kongCredType in code and allowing users to use either during a deprecation period. This requires providing two example webhook creation scripts, one which includes the objectSelector and one that doesn't (the latter is the same as the current one). Users that wish to continue using kongCredType will not get the benefits of non-credential Secrets being filtered out, but will be able to convert their Secrets gradually.
  • Providing a script to extract kongCredType from Secrets, remove the key, and add the appropriate label.

The objectSelector documentation is a bit confusing. https:/Kong/charts/blob/8af6326aa3a0ef7aac1b683f6438cd7f6e8488af/charts/kong/templates/admission-webhook.yaml#L37-L42 shows an example, though note that you'll need to use the In operator.

Acceptance Criteria

  • The admission controller does not ingest non-credential Secrets.
  • The admission controller does ingest Secrets with the appropriate label.
  • The parser can still determine the credential type using the new label.
@mflendrich
Copy link
Contributor

Note that this is very likely to end up being a breaking change, so if a PR comes for this change, we might need to be careful about when to release this.

@shaneutt shaneutt added the area/maintenance (breaking) Maintenance requiring backwards-incompat changes (used to track the need for a new major release) label Jul 26, 2022
@shaneutt shaneutt modified the milestones: KIC Next Major Release, Release v3.0.0 Aug 12, 2022
@mflendrich
Copy link
Contributor

Agreed that @rainest will split this into an issue for v2.x (implement support for type label in Secrets in parallel with kongCredType) and a separate issue for a breaking change to come in a major version (drop kongCredType in favor of already supported labels)

@rainest
Copy link
Contributor Author

rainest commented Jul 12, 2023

Minor note to self: I briefly thought this was mooted by the fix for #2868, but it isn't, as this concerns the admission webhook, not the controllers. We do still want this to avoid overly-aggressive admission checks on Secrets we don't care about.

@mflendrich mflendrich added breaking change and removed area/maintenance (breaking) Maintenance requiring backwards-incompat changes (used to track the need for a new major release) labels Sep 26, 2023
@rainest
Copy link
Contributor Author

rainest commented Oct 11, 2023

For migration, jq can parse existing credential secrets and print a list of commands to update them:

kubectl get secret -A -ojson | jq -r '.items[] | select(.data.kongCredType != null) | "kubectl label secret -n \(.metadata.namespace) \(.metadata.name) konghq.com/credential=\(.data.kongCredType | @base64d )"'

or without global namespace access:

for namespace in default other;  do kubectl get secret -n $namespace -ojson | jq -r '.items[] | select(.data.kongCredType != null) | "kubectl label secret -n \(.metadata.namespace) \(.metadata.name) konghq.com/credential=\(.data.kongCredType | @base64d )"'; done

This finds all secrets with the kongCredType field and prints a kubectl command that will apply a label whose value matches the field's.

$ kubectl get secret -A
NAMESPACE        NAME                     TYPE                            DATA   AGE
default          consumer-1-key-auth      Opaque                          2      17h
default          consumer-2-key-auth      Opaque                          2      17h
default          consumer-3-key-auth      Opaque                          2      17h
default          consumer-4-key-auth      Opaque                          2      17h
default          consumer-5-key-auth      Opaque                          2      17h
kube-system      bootstrap-token-abcdef   bootstrap.kubernetes.io/token   6      17h
metallb-system   memberlist               Opaque                          1      17h
metallb-system   webhook-server-cert      Opaque                          4      17h
other            consumer-10-key-auth     Opaque                          2      17h
other            consumer-6-key-auth      Opaque                          2      17h
other            consumer-7-key-auth      Opaque                          2      17h
other            consumer-8-key-auth      Opaque                          2      17h
other            consumer-9-key-auth      Opaque                          2      17h

$ kubectl get secret -A -ojson | jq -r '.items[] | select(.data.kongCredType != null) | "kubectl label secret -n \(.metadata.namespace) \(.metadata.name) konghq.com/credential=\(.data.kongCredType | @base64d )"'
kubectl label secret -n default consumer-1-key-auth konghq.com/credential=key-auth
kubectl label secret -n default consumer-2-key-auth konghq.com/credential=key-auth
kubectl label secret -n default consumer-3-key-auth konghq.com/credential=key-auth
kubectl label secret -n default consumer-4-key-auth konghq.com/credential=key-auth
kubectl label secret -n default consumer-5-key-auth konghq.com/credential=key-auth
kubectl label secret -n other consumer-10-key-auth konghq.com/credential=bee-auth
kubectl label secret -n other consumer-6-key-auth konghq.com/credential=bee-auth
kubectl label secret -n other consumer-7-key-auth konghq.com/credential=bee-auth
kubectl label secret -n other consumer-8-key-auth konghq.com/credential=bee-auth
kubectl label secret -n other consumer-9-key-auth konghq.com/credential=bee-auth

$ kubectl label secret -n default consumer-2-key-auth konghq.com/credential=key-auth
secret/consumer-2-key-auth labeled

Alternatives are building our own tool to do this or to build a tool that both finds and modifies resources. IMO jq is ubiquitous enough that creating and distributing our own standalone tool probably isn't worth it.

Outputting kubectl commands to be exceuted isn't perfect for all management strategies, but I don't think a tool that makes the modification itself would help. CD systems that lay out the secrets up front aren't amenable to a tool that modifies resources in place either. I don't know offhand how to make the transition smoother for those since I don't have a good handle on their inputs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants