[RS-2546] Update operator to deploy waf-http-filter in Enterprise (#4032) #4119
Conversation
|
Might be good to edit the default title to include |
LorcanMcVeigh
changed the title
…gera#4032) * Start a test for new behaviour * Use privileged PSS for tigera-gateway ns * Enable EnvoyPatchPolicy * Deploy and configure waf-http-filter * Fix codegen * Fix capitalisation * Support custom envoy proxy * Add comment for PSSPrivileged * Check volumes are set as expected --------- Co-authored-by: Antony Guinard <[email protected]>
LorcanMcVeigh
force-pushed
the
lorcan/waf-http-filter
branch
from
60c65b2 to
8399c8b
Compare
|
@LorcanMcVeigh With this cherry pick, have you checked for any remaining differences (for Gateway API code) between the release-v1.39 and master branches? Just in case there are any further PRs on master that will also need to be picked. |
|
|
||
| // Setup WAF HTTP Filter on Enterprise. | ||
| if pr.cfg.Installation.Variant == operatorv1.TigeraSecureEnterprise { | ||
| // The WAF HTTP filter is not supported when the envoy proxy is deployed as a DaemonSet |
There was a problem hiding this comment.
This restriction needs to be documented.
- One place, I think, would be under
GatewayKindhere: https://docs.tigera.io/calico-enterprise/3.22/reference/installation/api#gatewayclassspec - Will there also be a dedicated doc page for GatewayWAF? If so, I guess it should be mentioned there too.
@LorcanMcVeigh Do you have tickets tracking this?
There was a problem hiding this comment.
Gateway WAF is currently documented in https://docs.tigera.io/calico-enterprise/3.22/threat/deploying-waf-ingress-gateway. Agree we need to add a note under GatewayKind.
| if initContainer.Name == wafHTTPFilter.Name { | ||
| hasWAFHTTPFilter = true | ||
| // Handle update | ||
| if initContainer.Image != wafHTTPFilter.Image { |
There was a problem hiding this comment.
Why only check the image? We may in future want to update the WAF sidecar in other ways, so it might be better for this code to support that.
There was a problem hiding this comment.
I think the operator should just update the whole container spec that it wants, without checking for image change.
There was a problem hiding this comment.
Let's address this in a follow up PR to keep changes minimal for the cherry-pick.
| if volume.Name == logsVolume.Name { | ||
| hasLogsVolume = true | ||
| // Handle update | ||
| if volume.VolumeSource.HostPath.Path != logsVolume.VolumeSource.HostPath.Path || volume.VolumeSource.HostPath.Type != logsVolume.VolumeSource.HostPath.Type { |
There was a problem hiding this comment.
As above, I don't think we need this check. Usual operator behaviour is just to blat the complete thing that it wants.
There was a problem hiding this comment.
Let's address this in a follow up PR to keep changes minimal for the cherry-pick.
|
@nelljerram At this point, this PR is simply to get the feature into the release it was meant to be apart of. These changes don't need to be in this release. We can make the changes to master and they can be picked up in the next release but I don't see the need for them to be in this cherry-pick or any follow-up cherry-pick. |
5 participants
Start a test for new behaviour
Use privileged PSS for tigera-gateway ns
Enable EnvoyPatchPolicy
Deploy and configure waf-http-filter
Fix codegen
Fix capitalisation
Support custom envoy proxy
Add comment for PSSPrivileged
Check volumes are set as expected
Description
Release Note
For PR author
make gen-filesmake gen-versionsFor PR reviewers
A note for code reviewers - all pull requests must have the following:
kind/bugif this is a bugfix.kind/enhancementif this is a a new feature.enterpriseif this PR applies to Calico Enterprise only.