-
Notifications
You must be signed in to change notification settings - Fork 5
Add support for RuleGroup CRD #6
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
Add support for RuleGroup CRD #6
Conversation
Skipping CI for Draft Pull Request. |
/test all |
1 similar comment
/test all |
Issue #, if available: prerequisite for WAFv2 [RuleGroup](aws-controllers-k8s/wafv2-controller#6) and [WebACL](aws-controllers-k8s/wafv2-controller#7) CRDs Description of changes: Some WAFv2 API fields have empty json specs `{}`, e.g. https://docs.aws.amazon.com/waf/latest/APIReference/API_AllQueryArguments.html For these type of fields, codegen currently errors out because it infers their gotypes as e.g. `AllQueryArguments *AllQueryArguments` but does not generate a `type AllQueryArguments struct` since the struct itself is empty, and not picked up by `newFieldRecurse` function. The solution proposed in this PR is to allow users to define `marker-shapes`, which instruct codegen to overwrite the type of these empty structs as `[]byte`, both when generating the APIs and when setting up the SDK. e.g. [generator.yaml](https://github.com/aws-controllers-k8s/wafv2-controller/blob/bb682409da8f96c5035783602b9d948c8cc8e21f/apis/v1alpha1/generator.yaml#L15-L24) for WAFv2, and inline: ``` empty_shapes: - All - Method - UriPath - QueryString - AllQueryArguments - RateLimitIP - RateLimitForwardedIP - RateLimitHTTPMethod - NoneAction ``` By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
bb68240
to
7ed2689
Compare
7ed2689
to
239a645
Compare
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.
Great stuff Tibi! i left few comments in line
apis/v1alpha1/types.go
Outdated
// use in web requests in an effort to bypass detection. | ||
type TextTransformation struct { | ||
Priority *int64 `json:"priority,omitempty"` | ||
Type *string `json:"type_,omitempty"` |
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.
We can rename this json tag to type
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.
Can you point me to a similar example 🙏🏻 ?
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.
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.
apis/v1alpha1/types.go
Outdated
// A rule statement that inspects for cross-site scripting (XSS) attacks. In | ||
// XSS attacks, the attacker uses vulnerabilities in a benign website as a vehicle | ||
// to inject malicious client-site scripts into other legitimate web browsers. | ||
XssMatchStatement *XssMatchStatement `json:"xssMatchStatement,omitempty"` |
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.
Should we add an exception in pkg/names
about Xss/XSS?
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.
apis/v1alpha1/types.go
Outdated
// A rule statement that inspects for malicious SQL code. Attackers insert malicious | ||
// SQL code into web requests to do things like modify your database or extract | ||
// data from it. | ||
SQLiMatchStatement *SQLiMatchStatement `json:"sqliMatchStatement,omitempty"` |
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.
SQLi
or SQLI
?
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.
SQLI
sounds good
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.
apis/v1alpha1/types.go
Outdated
// | ||
// Provide the JA3 fingerprint string from the logs in your string match statement | ||
// specification, to match with any future requests that have the same TLS configuration. | ||
JA3Fingerprint *JA3Fingerprint `json:"jA3Fingerprint,omitempty"` |
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.
s/jA3/JA3/g
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.
27267cd
to
d9616ee
Compare
/test all |
/retest |
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.
Super neat 👍
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: a-hilaly, TiberiuGC 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 |
Issue #, if available: aws-controllers-k8s/community#1300
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.