Merge nginxaas changes #2
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NLB-4470: Setup the repo See merge request f5/nginx/nginxazurelb/nlk!1
The primary intent behind this is to keep retrying updates which may be made before the controlplane has registered the existence of a named upstream in the customer's NGINX configuration.
NLB-4655 NLK will retry a work item to update upstreams indefinitely See merge request f5/nginx/nginxazurelb/nlk!3
NLB-5267: Rename nlk -> nginxaas-operator See merge request f5/nginx/nginxazurelb/nlk!4
Disabling caching temporarily to figure out the issue and we can re-enable it after that.
NLB-5341: Disable dep caching See merge request f5/nginx/nginxazurelb/nginxaas-operator!8
NOTE: This commit was accidentally missed out in the first iteration of this fork and is present in upstream: https://github.com/nginxinc/nginx-loadbalancer-kubernetes/blob/main/internal/configuration/settings.go#L189 The code needs to move forward to start the watchers and health server (side note: we should also think about the ordering of these at some point) and not running the infomer in a go routine prevents the program from further execution until the context is canceled. In the current iteration on main, the controller is stuck on the informer, and then k8s kills the service and restarts it since the health server is not up.
Run the informer in go routine See merge request f5/nginx/nginxazurelb/nginxaas-operator!9
The user specifies the ingress service whose events the application should watch through setting the "service-annotation-match" annotation on the application's config map. Only events with a matching service annotation will be passed by the watcher to the handlers. The informer now listens to events from all namespaces. This frees the end user from the restriction of only using the nginx ingress controller.
NLB-4617 Watcher filters events by annotation on ingress service name See merge request f5/nginx/nginxazurelb/nginxaas-operator!6
…eam name The port name should now be formatted like this: "http-tea", where the first part of the string is the context type (either "http" or "stream") and the second part of the string after the hyphen is the name of the upstream.
NLB-4823 Translator assumes that port names provide context and upstream name See merge request f5/nginx/nginxazurelb/nginxaas-operator!5
We need to be able to publish the operator to dockerhub in order to be publicly available for customers. Following what we have in ARP as a release strategy where a release tag action would publish the image to dockerhub.
NLB-5282: Allow images to be pushed to Dockerhub See merge request f5/nginx/nginxazurelb/nginxaas-operator!11
Remove unneeded file See merge request f5/nginx/nginxazurelb/nginxaas-operator!7
go test does not have a good way to capture unit-test coverage as part of test runs. This commit captures the error code of the unit test run, runs the coverage generation and then exits based on the test status.
Capture code coverage See merge request f5/nginx/nginxazurelb/nginxaas-operator!12
We want to avoid running tests several times on main as it increases the time it takes to publish an image on regular merges. Instead, we can run these tests on a schedule on main and capture code coverage from it.
Run coverage in a separate job See merge request f5/nginx/nginxazurelb/nginxaas-operator!13
NLB-5360 Upgraded nginx-plus client to v 1.2.2 See merge request f5/nginx/nginxazurelb/nginxaas-operator!15
NLB-6294 The synchronizer uses a typed rate-limited workqueue See merge request f5/nginx/nginxazurelb/nginxaas-loadbalancer-kubernetes!75
We don't really have a need to handle multi arch images yet and all our local workflows are tied to using AKS. While that might change in the future, for the time being, we want to make sure to build and publish linux/amd64 images to match the test envs that we have.
Force linux builds for the product See merge request f5/nginx/nginxazurelb/nginxaas-loadbalancer-kubernetes!76
Added documents on how to release NLK See merge request f5/nginx/nginxazurelb/nginxaas-loadbalancer-kubernetes!78
These functions were being used to create IDs that were not really necessary for the business logic and which were generating security alerts because of weak cryptography techniques.
The http client is processing requests created by the nginx plus client library, and that library should always include a sensible number of headers. But the lack of change on the number of headers was causing security vulnerability flags to be raised over denial of service resource exhaustion attacks.
Fixes See merge request f5/nginx/nginxazurelb/nginxaas-loadbalancer-kubernetes!79
NLB-6372: enable scanning on merge requests, merged to main and tags with v* Closes NLB-6372 See merge request f5/nginx/nginxazurelb/nginxaas-loadbalancer-kubernetes!80
Go cache in the CI is seeded in the project working directory. We should skip the mod cache from lint/formatting as it's upstream code and there are high chances of the linting failing as upstream lint rules != our lint rules.
Re-enable dependency caching See merge request f5/nginx/nginxazurelb/nginxaas-loadbalancer-kubernetes!81
unit test flake fix and go version upgrade See merge request f5/nginx/nginxazurelb/nginxaas-loadbalancer-kubernetes!82
…inx hosts In order for the nginx-hosts yaml field to be parsed correctly by viper the template needs to: 1. not put double quotes around the value (this causes viper to interpret it as a string) 2. render it as a JSON array rather than a go representation of a slice.
Fixed up helm templating for nginx-hosts param to support multiple nginx hosts See merge request f5/nginx/nginxazurelb/nginxaas-loadbalancer-kubernetes!83
The biggest change here is to remove most the TLS modes to enable mTLS and self-signed certificates. Product decided that this was too complex and there was not enough user demand for most of these options. We decided to pare down the code and remove tests that were no longer well maintained. The remaining configuration allows users to toggle a single switch: whether to make the http client verify the NGINX host's certificate chain and host name if https is being used. If the user wishes to enable https with self-signed certs they can use the "skip-verify-tls" setting to allow this. The default behavior is to perform this verification. We are maintaining the deprecated "no-tls" and "ca-tls" inputs for NGINXaaS backwards comptability reasons. The "no-tls" setting name was highly misleading, because all it did was disable TLS verification: it DID NOT disable TLS altogether in https mode. Similarly, the "ca-tls" setting did not enable TLS itself. TLS is enabled by default when the URL of the NGINX host includes the https protocol. The user setting merely enforced the verification of the certificate chain and host as described above.
NLB-6295 Simplified user configuration of TLS modes See merge request f5/nginx/nginxazurelb/nginxaas-loadbalancer-kubernetes!77
Now that the plus go client allows users to check the http status code of the error, handle the upstream not found case by doing nothing.
NLB-6754 When deleting upstream servers handle upstream not found error See merge request f5/nginx/nginxazurelb/nginxaas-loadbalancer-kubernetes!84
This is a go anti-pattern
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This MR merges all the code added by the NGINXaaS project within its private fork of the original open source repo.