-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Cilium wireguard support #12158
Cilium wireguard support #12158
Conversation
@olemarkus: GitHub didn't allow me to request PR reviews from the following users: dntosas. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation and CLI refer to the "ciliumpassword" secret as the "Cilium IPsec configuration". It looks like the code requires and provisions this for Wireguard as well. The terminology in the documentation/CLI should be updated.
if fi.BoolValue(v.EnableL7Proxy) { | ||
allErrs = append(allErrs, field.Forbidden(fldPath.Child("encryptionType"), "Cilium EncryptionType=WireGuard cannot work with L7 Proxy enabled.")) | ||
} | ||
|
||
// Cilium with Wireguard integration follow-up --> https://github.com/cilium/cilium/issues/15462. | ||
// The following rule of validation should be deleted as this combination | ||
// will be supported on future releases of Cilium (>= v1.11.0). | ||
if fi.BoolValue(v.EnableL7Proxy) { | ||
allErrs = append(allErrs, field.Forbidden(fldPath.Child("enableL7Proxy"), "L7 proxy cannot be enabled if wireguard is enabled.")) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two validations are redundant with each other. One should be removed.
Ah, if the ciliumpassword secret isn't needed for Wireguard, then kOps shouldn't require it to exist |
kops/upup/pkg/fi/cloudup/apply_cluster.go Lines 363 to 370 in cd1aa1a
Needs to test the encryption type. |
In this commit, we enable users to choose WireGuard as their prefered encryption type, leveraging this new feature from Cilium. Ref: https://cilium.io/blog/2021/05/20/cilium-110#wireguard Signed-off-by: dntosas <ntosas@gmail.com>
d3447af
to
3b78a29
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: johngmyers The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This is a continuation of #11786 which went inactive.
/cc @dntosas