Skip to content

Conversation

mauriciovasquezbernal
Copy link
Contributor

"kubectl get secret -n kube-system cilium-ipsec-keys" outputs two lines
with "keys:", one is the real key data and other is a "managedField":

$ kubectl get secret -n kube-system cilium-ipsec-keys -o yaml | grep keys:
keys: MyByZmM0MTA2KGdjbShhZXMpKSA3ZTE1YmZlNmQyZjczNGUzZmQ0YTEzM2FlZDU2MGQwMjEzZjBjNmRmIDEyOA==
f:keys: {}

It makes the whole command to get the key id to fail:

$ KEYID=$(kubectl get secret -n kube-system cilium-ipsec-keys -o yaml|grep keys: | awk '{print $2}' | base64 -d | awk '{print $1}')
base64: invalid input

This will be fixed in next Kubernetes release
(kubernetes/kubernetes#96878), in the meanwhile
we can just use "-m 1" for grep to make it only return the first result.

Fixes: 4ea52ae ("cilium: encryption, docs key updates")

Signed-off-by: Mauricio Vásquez mauricio@accuknox.com
Signed-off-by: Mauricio Vásquez mauricio@kinvolk.io

@mauriciovasquezbernal mauriciovasquezbernal requested a review from a team as a code owner March 16, 2021 15:03
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 16, 2021
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Thanks a lot!
Looks good, but please find a suggestion below.

@qmonnet qmonnet added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact. labels Mar 16, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 16, 2021
@maintainer-s-little-helper
Copy link

Commit b6b767b8465bfbd1729550270ed5b954001874ae does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Mar 16, 2021
"kubectl get secret -n kube-system cilium-ipsec-keys" outputs two lines
with "keys:", one is the real key data and other is a "managedField":

$ kubectl get secret -n kube-system cilium-ipsec-keys -o yaml | grep keys:
  keys: MyByZmM0MTA2KGdjbShhZXMpKSA3ZTE1YmZlNmQyZjczNGUzZmQ0YTEzM2FlZDU2MGQwMjEzZjBjNmRmIDEyOA==
        f:keys: {}

It makes the whole command to get the key id to fail:

$ KEYID=$(kubectl get secret -n kube-system cilium-ipsec-keys -o yaml|grep keys: | awk '{print $2}' | base64 -d | awk '{print $1}')
base64: invalid input

This will be fixed in next Kubernetes release
(kubernetes/kubernetes#96878), in the meanwhile
just use a regular expression in awk to match "keys:" at the begining.

Fixes: 4ea52ae ("cilium: encryption, docs key updates")

Signed-off-by: Mauricio Vásquez <mauricio@accuknox.com>
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
@mauriciovasquezbernal mauriciovasquezbernal force-pushed the mauricio/fix-encryption-key-rotation-command branch from b6b767b to 537bf79 Compare March 16, 2021 16:18
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Mar 16, 2021
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@qmonnet qmonnet added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 16, 2021
@joestringer joestringer merged commit 458c623 into cilium:master Mar 17, 2021
@mauriciovasquezbernal mauriciovasquezbernal deleted the mauricio/fix-encryption-key-rotation-command branch March 17, 2021 01:06
This was referenced Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants