* images/kube-webhook-certgen/rootfs/pkg/k8s: return err from functions
Initially only from some to preserve existing behavior.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs: make patching return error
So we don't call log.Fatal in so many places, which makes code testable.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs/pkg/k8s: require context
So initialize top-level contexts in tests and CLI, then pass them around
all the way down, so there is an ability e.g. to add timeouts to patch
operations, if needed and to follow general conventions.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs/pkg/k8s: support patching APIService
APIService object is very similar to MutatingWebhookConfiguration and
ValidatingWebhookConfiguration objects, so support for patching it
shouldn't be too much of a burden.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs/cmd: use new patch API
So old function PatchWebhookConfigurations can be unexported and CLI can
be extended to also support patching APIService.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs/pkg/k8s: unexport old patch function
PatchObjects should be now used instead.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs: add .gitignore
To ignore manually built binaries during development process.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs/cmd: test patching
By adding a PatchConfig and Patch function, it is now possible to test
logic of flag validation, which was previously tied to CLI options.
This commit adds nice set of tests covering existing logic.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs/cmd: improve formatting
Those strings will be changed anyway in future commits, so at first we
can properly capitalize used names.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs/cmd: support patching APIService
As logic for creating a CA certificate and patching an object is almost
the same for both webhook configuration and API services, this commit
adds support to kube-webhook-certgen CLI to also patch APIService
objects, so they can be served over TLS as well.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs: pass failure policy by value
k8s.k8s.patchWebhookConfigurations() always dereferences it and we do
not do a nil check, so the code may panic in some conditions, so it's
safer to just pass it by value, as it's just a wrapped string.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
it has two important bugfix:
1. should force convert weight to a number since it may cause dead loop
when weight is a string type "0".
2. out-of-bounds memory writing may happen in chash_point_sort.
Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
Since kube-lego has not been maintained in quite a while,
I thought it would be best to remove the documentation about it
and replace it with information about cert-manager.
* added another documentation example
* added end of file newline
* Revert "added end of file newline"
This reverts commit 2d196ffba3.
* added another documentation example
* images/kube-webhook-certgen/rootfs/README.md: remove trailing whitespace
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs: improve code formatting
Automatically using gofumpt.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs: remove executable bits from files
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs/cmd: remove unreachable code
log.Fatal(|f) will alread call os.Exit(1), so this code is never
reached.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs/pkg/k8s: fix unit tests
Right now they fail as everything else migrated from using v1beta1 to
v1.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs: create clientset in cmd package
So one can easily mock the client, without touching unexported parts of
the code and to soften the dependency between CLI code (kubeconfig
path).
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs/cmd: simplify bool logic
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs/pkg/k8s: improve formatting
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs/pkg/k8s: improve variable names
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs/pkg/k8s: refactor a bit
Move patching logic to separate functions.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
* images/kube-webhook-certgen/rootfs/pkg/k8s: fix error log messages
In patchMutating() function, log messages were waying still patching
validating webhook.
Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
- Add github action test-image-build
- Filters the images folder
and checks for changes
- If the changes are done then the
make build would be performed
* Fix old tag of custom error pages used in example
* Move nginx-errors to k8s registry
Since the setup for the custom-error-messages was really different from
the other images that are build using cloudbuild, I changed it to "fit
in better"
* Use Go version 1.17 for custom-error-pages
Since Go >= 1.16 required the use of modules, I also initialized the module using the name k8s.io/ingress-nginx/custom-error-pages
It is possible to change this behavior on an ingress level, which works
well when you only have a few of them. When running several dozen
ingress and with a high change rate of running pods it makes it easier
to define this configuration on a global level.
This change is completely backwards compatible, only adding the
possibility of defining a new key in the configmap.
<!--- Provide a general summary of your changes in the Title above --->
<!--- Why is this change required? What problem does it solve? -->
Introduces the CLI command flag `--disable-full-test`
By default, it doesn't alter the current behavior of the tests performed by the admission controller.
With or Without the flag, a full checkOverlap is actioned, without any alteration
and the object `pcfg` is created with the whole set of ingreses.
If the flag is set to true, it does manipulate the size of `pcfg` up to the content of $this single ingress.
This is achieved by overriding pcfg content by just the last slice that got recently appended to the object `ings`
```
if n.cfg.DisableFullValidationTest {
_, _, pcfg = n.getConfiguration(ings[len(ings)-1:])
}
```
The following steps of generateTemplate and testTemplate are significally reduced to a signle scenario
```
content, err := n.generateTemplate(cfg, *pcfg)
...
err = n.testTemplate(content)
```
This flag doesn't avoid the proper testing of collisions, neither bad syntaxis within the rendered
configuration of the ingress.
But it does eliminate a scenario, which I wasn't able to produce, where by for some reason even proper rendering
and valid values, without collisions of host/path may end into an invalid nginx.conf
The reasoning for this Feature is:
- Test duration increases by the number of ingresses in the cluster.
- File size grows to very important numbers 150-200Mb on clusters with just 2000~ ingresses.
- Tests in that scenario, takes approximately 20s using the last 0.48.1 improvements
- Produces a considerable memory consumption, as well as CPU, compute, that affects directly the containers
that serve traffic.
Since the flag is trully optional, and by default is disabled I fell as a good thing to have that can definitively
help on large-scale scenarios that still want to have a reasonable set of tests in place at a lower cost.
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [X ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->
Tested with the build kit the following scenarios on a cluster with 1000~ ingresses:
- With Flag Disabled or Flag, not present (current status as per 0.48.1)
collision scenario (wrong snippet content):
`kubectl apply -f ../collision-syntax.yaml 0.18s user 0.05s system 3% cpu 6.639 total`
collisions scenario (duplicated host):
`kubectl apply -f ../collision-host.yaml 0.17s user 0.05s system 3% cpu 6.245 total`
create/update:
`kubectl apply -f ing-215.yaml 0.16s user 0.05s system 3% cpu 5.845 total`
- With Flag Enabled (true):
collision scenario (wrong snippet content):
`kubectl apply -f ../collision.yaml 0.18s user 0.02s system 57% cpu 0.347 total`
collision scenario (duplicated host):
`kubectl apply -f ../collision.yaml 0.21s user 0.06s system 85% cpu 0.318 total`
create/update:
`kubectl apply -f ing-973.yaml 0.17s user 0.03s system 72% cpu 0.271 total`
As part of the test, I did verified that the created nginx for the test was of a smaller size, and that it didnt affect negatively the final nginx.conf (of a much larger side) where this was merged by the next steps in place after the validation. I couldn't observe any other change in the behaviour and so far the routine looks simple and non harmful.
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [x] My change requires a change to the documentation.
- [x] I have updated the documentation accordingly.
- [x] I've read the [CONTRIBUTION](https://github.com/kubernetes/ingress-nginx/blob/main/CONTRIBUTING.md) guide
- [ ] I have added tests to cover my changes.
- [ ] All new and existing tests passed.
For the test part, I would need to understand the placement and test case that this would require, I wasn't able to see an existing scenario for this
Normally Ingress sinchronization for Services is triggered when
corresponding Service's Endpoints are added, deleted or modified.
Services of type ExternalName, however, do not have any endpoints
and hence do not trigger Ingress synchronization as only Update
events are being watched. This commit makes sure that Update and
Delete Service events also enqueue a syncIngress task.
* Change statusSync.runningAddresses() return type
Previously, this method returning a string slice containing the resolved
IP addresses / FQDNs to sync onto the Ingress. It was then converted
just before use into a slice of LoadBalancerIngresses.
This commit changes this logic so that this method generates
LoadBalancerIngress objects directly, and returns these. This has two
main benefits:
- Future work in syncing _both_ hostname and IP, or any other fields
that may be used in future (eg Ports), is now supported.
- There is less need to rely on net.ParseIP() to determine if a value is
an IP address or Hostname, as this can be correctly assigned at
generation time based on where each value came from.
* Sync both IP and Hostname to Ingress Status
Previously, if the IP address was set on a PublishService's
LoadBalancerIngress entries, only that would be synced. Hostname was
only synced as a fallback when the IP address was missing.
Now, both fields are checked independantly and both are synced if
present.
* Fix indentation of nested list in AuthTLS annotations
Also, put `<annotation>`: <description text>` on a single line in
Markdown markup, which will match what gets rendered eventually.
On the other hand, for the line on auth-tls-secret (This annotation
expects the Secret name in the form "namespace/secretName"), its
Markdown markup suggests that the author wanted the line to start on its
own line, but currently this gets rendered on the same line. It's nice
for this to be on its own line, since it's kind of a "note" about the
annotation syntax. Format/indent the markup appropriately so that it
shows up on its line.
* Fix indentation of nested list in CORS annotations
Also, put `<annotation>`: <description text>` on a single line in
Markdown markup, which will match what gets rendered eventually.
On the other hand, for lines noting the allowed characters (This is a
multi-valued field...), its Markdown markup suggests that the author
wanted the line to start on its own line, but currently this gets
rendered on the same line. It's nice for this to be on its own line,
since it's kind of a "note" about the annotation syntax. Format/indent
the markup appropriately so that it shows up on its line.
* Replace f.HTTPTestClientWithTLSConfig() in AuthTLS E2E, the odd one out for requests without client certs
* Demonstrate and document that auth-tls-secret enables the other AuthTLS annotations like verify client, depth
* Split E2E for auth-tls-error-page and *-pass-certificate-to-upstream
* Update to the base nginx image
* Revert "Update to the base nginx image"
This reverts commit ad43c1d060.
* Update test runner image
* correcting the sha and version of e2e test runner images
* Update to the base nginx image
* Revert "Update to the base nginx image"
This reverts commit ad43c1d060.
* Updated cloudbuild to increase build timeout value
