Skip to content

Commit

Permalink
Review feedback and add CI check
Browse files Browse the repository at this point in the history
  • Loading branch information
ciarams87 committed Jul 27, 2023
1 parent 3166dce commit b3dbca2
Show file tree
Hide file tree
Showing 19 changed files with 80 additions and 106 deletions.
5 changes: 4 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,15 @@ jobs:
- name: Check if go.mod and go.sum are up to date
run: go mod tidy && git diff --exit-code -- go.mod go.sum

- name: Check if generated files are up to date
- name: Check if generated go files are up to date
run: make generate && git diff --exit-code

- name: Check if njs-modules yaml is up to date
run: make generate-njs-yaml && git diff --exit-code

- name: Check if generated manifests are up to date
run: make generate-manifests && git diff --exit-code

unit-tests:
name: Unit Tests
runs-on: ubuntu-22.04
Expand Down
8 changes: 4 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ TARGET ?= local## The target of the build. Possible values: local and container
KIND_KUBE_CONFIG=$${HOME}/.kube/kind/config## The location of the kind kubeconfig
OUT_DIR ?= $(shell pwd)/build/out## The folder where the binary will be stored
ARCH ?= amd64## The architecture of the image and/or binary. For example: amd64 or arm64
override HELM_TEMPLATE_COMMON_ARGS += --set creator=template --set nameOverride=nginx-gateway ## The common options for the Helm template command.
override HELM_TEMPLATE_EXTRA_ARGS += --set service.create=false ## The options to be passed to the full Helm templating command only.
override HELM_TEMPLATE_COMMON_ARGS += --set creator=template --set nameOverride=nginx-gateway## The common options for the Helm template command.
override HELM_TEMPLATE_EXTRA_ARGS_FOR_ALL_MANIFESTS_FILE += --set service.create=false## The options to be passed to the full Helm templating command only.
override DOCKER_BUILD_OPTIONS += --build-arg VERSION=$(VERSION) --build-arg GIT_COMMIT=$(GIT_COMMIT) --build-arg DATE=$(DATE)## The options for the docker build command. For example, --pull

.DEFAULT_GOAL := help
Expand Down Expand Up @@ -121,11 +121,11 @@ debug-container: debug-build container ## Build container with debug binary

.PHONY: generate-manifests
generate-manifests: ## Generate manifests using Helm.
helm template nginx-gateway $(CHART_DIR) $(HELM_TEMPLATE_COMMON_ARGS) $(HELM_TEMPLATE_EXTRA_ARGS) -n nginx-gateway | cat $(strip $(MANIFEST_DIR))/namespace.yaml - > $(strip $(MANIFEST_DIR))/nginx-gateway.yaml
helm template nginx-gateway $(CHART_DIR) $(HELM_TEMPLATE_COMMON_ARGS) $(HELM_TEMPLATE_EXTRA_ARGS_FOR_ALL_MANIFESTS_FILE) -n nginx-gateway | cat $(strip $(MANIFEST_DIR))/namespace.yaml - > $(strip $(MANIFEST_DIR))/nginx-gateway.yaml
helm template nginx-gateway $(CHART_DIR) $(HELM_TEMPLATE_COMMON_ARGS) -n nginx-gateway -s templates/deployment.yaml > conformance/provisioner/static-deployment.yaml
helm template nginx-gateway $(CHART_DIR) $(HELM_TEMPLATE_COMMON_ARGS) -n nginx-gateway -s templates/service.yaml > $(strip $(MANIFEST_DIR))/service/loadbalancer.yaml
helm template nginx-gateway $(CHART_DIR) $(HELM_TEMPLATE_COMMON_ARGS) --set service.annotations.'service\.beta\.kubernetes\.io\/aws-load-balancer-type'="nlb" -n nginx-gateway -s templates/service.yaml > $(strip $(MANIFEST_DIR))/service/loadbalancer-aws-nlb.yaml
helm template nginx-gateway $(CHART_DIR) $(HELM_TEMPLATE_COMMON_ARGS) --set service.type=NodePort -n nginx-gateway -s templates/service.yaml > $(strip $(MANIFEST_DIR))/service/nodeport.yaml
helm template nginx-gateway $(CHART_DIR) $(HELM_TEMPLATE_COMMON_ARGS) --set service.type=NodePort --set service.externalTrafficPolicy="" -n nginx-gateway -s templates/service.yaml > $(strip $(MANIFEST_DIR))/service/nodeport.yaml

.PHONY: dev-all
dev-all: deps fmt njs-fmt vet lint unit-test njs-unit-test ## Run all the development checks
4 changes: 2 additions & 2 deletions conformance/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ preload-nginx-container: ## Preload NGINX container on configured kind cluster
kind load docker-image $(NGINX_IMAGE)

.PHONY: update-nkg-manifest
update-nkg-manifest:
cd .. && make generate-manifests HELM_TEMPLATE_EXTRA_ARGS="--set nginxGateway.kind=skip" HELM_TEMPLATE_COMMON_ARGS="--set nginxGateway.image.repository=$(NKG_PREFIX) --set nginxGateway.image.tag=$(NKG_TAG) --set nginxGateway.image.pullPolicy=Never" && cd -
update-nkg-manifest: ## Update the NKG deployment manifest image name and imagePullPolicy
cd .. && make generate-manifests HELM_TEMPLATE_EXTRA_ARGS_FOR_ALL_MANIFESTS_FILE="--set nginxGateway.kind=skip" HELM_TEMPLATE_COMMON_ARGS="--set nginxGateway.image.repository=$(NKG_PREFIX) --set nginxGateway.image.tag=$(NKG_TAG) --set nginxGateway.image.pullPolicy=Never" && cd -

.PHONY: build-nkg-image
build-nkg-image: ## Build NKG container and load it and NGINX container on configured kind cluster
Expand Down
13 changes: 7 additions & 6 deletions conformance/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ run-conformance-tests Run conformance tests
undo-manifests-update Undo the changes in the manifest files
uninstall-nkg Uninstall NKG on configured kind cluster and undo manifest changes
update-go-modules Update the gateway-api go modules to latest main version
update-nkg-manifest Update the NKG deployment manifest image name and imagePullPolicy
```

**Note:** The following variables are configurable when running the below `make` commands:
Expand All @@ -49,11 +50,11 @@ update-go-modules Update the gateway-api go modules to latest main
| GATEWAY_CLASS | nginx | The gateway class that should be used for the tests |
| SUPPORTED_FEATURES | HTTPRoute,HTTPRouteQueryParamMatching, HTTPRouteMethodMatching,HTTPRoutePortRedirect, HTTPRouteSchemeRedirect | The supported features that should be tested by the conformance tests. Ensure the list is comma separated with no spaces. |
| EXEMPT_FEATURES | ReferenceGrant | The features that should not be tested by the conformance tests |
| NGINX_IMAGE | as defined in the provisioner/static-deployment.yaml file | The NGINX image for the NKG deployments |
| NKG_MANIFEST | ../deploy/manifests/nginx-gateway.yaml | The location of the NKG manifest |
| SERVICE_MANIFEST | ../deploy/manifests/service/nodeport.yaml | The location of the NKG Service manifest |
| STATIC_MANIFEST | provisioner/static-deployment.yaml | The location of the NKG static deployment manifest |
| PROVISIONER_MANIFEST | provisioner/provisioner.yaml | The location of the NKG provisioner manifest |
| NGINX_IMAGE | as defined in the provisioner/static-deployment.yaml file | The NGINX image for the NKG deployments |
| NKG_MANIFEST | ../deploy/manifests/nginx-gateway.yaml | The location of the NKG manifest |
| SERVICE_MANIFEST | ../deploy/manifests/service/nodeport.yaml | The location of the NKG Service manifest |
| STATIC_MANIFEST | provisioner/static-deployment.yaml | The location of the NKG static deployment manifest |
| PROVISIONER_MANIFEST | provisioner/provisioner.yaml | The location of the NKG provisioner manifest |

### Step 1 - Create a kind Cluster

Expand Down Expand Up @@ -85,7 +86,7 @@ You can optionally skip the actual *build* step.
make install-nkg-local-no-build
```

> Note: If choosing this option, the following step *must* be completed manually *before* the build step:
> Note: If choosing this option, the following step *must* be completed manually *before* you build the image:
```makefile
make update-nkg-manifest NKG_PREFIX=<nkg_repo_name> NKG_TAG=<nkg_image_tag>
Expand Down
4 changes: 1 addition & 3 deletions conformance/provisioner/static-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,17 @@ metadata:
labels:
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
app.kubernetes.io/version: "0.5.0"
app.kubernetes.io/version: "edge"
spec:
# We only support a single replica for now
replicas: 1
selector:
matchLabels:
app: nginx-gateway
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
template:
metadata:
labels:
app: nginx-gateway
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
spec:
Expand Down
2 changes: 1 addition & 1 deletion deploy/helm-chart/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: nginx-kubernetes-gateway
description: NGINX Kubernetes Gateway
type: application
version: 0.1.0
appVersion: "0.5.0"
appVersion: "edge"
home: https://github.com/nginxinc/nginx-kubernetes-gateway
icon: https://raw.githubusercontent.com/nginxinc/nginx-kubernetes-gateway/tree/main/deploy/helm-chart/chart-icon.png
sources:
Expand Down
5 changes: 1 addition & 4 deletions deploy/helm-chart/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ This chart deploys the NGINX Kubernetes Gateway in your Kubernetes cluster.
- [Helm 3.0+](https://helm.sh/docs/intro/install/)
- [kubectl](https://kubernetes.io/docs/tasks/tools/)

> Note: NGINX Kubernetes Gateway can only run in the `nginx-gateway` namespace. This limitation will be addressed in
the future releases.

### Installing the Gateway API resources

> Note: The Gateway API resources from the standard channel (the CRDs and the validating webhook) must be installed
Expand Down Expand Up @@ -115,7 +112,7 @@ The following tables lists the configurable parameters of the NGINX Kubernetes G
|`nginxGateway.image.pullPolicy` | The `imagePullPolicy` for the NGINX Kubernetes Gateway image. | Always |
|`nginxGateway.gatewayClassName` | The name of the GatewayClass for the NGINX Kubernetes Gateway deployment. | nginx |
|`nginxGateway.gatewayControllerName` | The name of the Gateway controller. The controller name must be of the form: DOMAIN/PATH. The controller's domain is k8s-gateway.nginx.org. | k8s-gateway.nginx.org/nginx-gateway-controller |
|`nginxGateway.kind` | The kind of the NGINX Kubernetes Gateway installation - currently, only deployment is supported. | deployment |
|`nginxGateway.kind` | The kind of the NGINX Kubernetes Gateway installation - currently, only Deployment is supported. | deployment |
|`nginx.image.repository` | The repository for the NGINX image. | nginx |
|`nginx.image.tag` | The tag for the NGINX image. | 1.25 |
|`nginx.image.pullPolicy` | The `imagePullPolicy` for the NGINX image. | Always |
Expand Down
2 changes: 0 additions & 2 deletions deploy/helm-chart/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,10 @@ spec:
replicas: 1
selector:
matchLabels:
app: nginx-gateway
{{- include "nginx-gateway.selectorLabels" . | nindent 6 }}
template:
metadata:
labels:
app: nginx-gateway
{{- include "nginx-gateway.selectorLabels" . | nindent 8 }}
spec:
containers:
Expand Down
2 changes: 1 addition & 1 deletion deploy/helm-chart/templates/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ spec:
type: {{ .Values.service.type }}
selector:
{{- include "nginx-gateway.selectorLabels" . | nindent 4 }}
ports:
ports: # Update the following ports to match your Gateway Listener ports
{{- if .Values.service.ports }}
{{ toYaml .Values.service.ports | indent 2 }}
{{ end }}
Expand Down
16 changes: 7 additions & 9 deletions deploy/manifests/nginx-gateway.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ metadata:
labels:
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
app.kubernetes.io/version: "0.5.0"
app.kubernetes.io/version: "edge"
annotations:
{}
---
Expand All @@ -25,7 +25,7 @@ metadata:
labels:
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
app.kubernetes.io/version: "0.5.0"
app.kubernetes.io/version: "edge"
data:
nginx.conf: |-
load_module /usr/lib/nginx/modules/ngx_http_js_module.so;
Expand All @@ -52,7 +52,7 @@ metadata:
labels:
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
app.kubernetes.io/version: "0.5.0"
app.kubernetes.io/version: "edge"
data:
httpmatches.js: |
const MATCHES_VARIABLE = 'http_matches';
Expand Down Expand Up @@ -271,7 +271,7 @@ metadata:
labels:
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
app.kubernetes.io/version: "0.5.0"
app.kubernetes.io/version: "edge"
rules:
- apiGroups:
- ""
Expand Down Expand Up @@ -330,7 +330,7 @@ metadata:
labels:
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
app.kubernetes.io/version: "0.5.0"
app.kubernetes.io/version: "edge"
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
Expand All @@ -349,19 +349,17 @@ metadata:
labels:
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
app.kubernetes.io/version: "0.5.0"
app.kubernetes.io/version: "edge"
spec:
# We only support a single replica for now
replicas: 1
selector:
matchLabels:
app: nginx-gateway
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
template:
metadata:
labels:
app: nginx-gateway
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
spec:
Expand Down Expand Up @@ -449,6 +447,6 @@ metadata:
labels:
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
app.kubernetes.io/version: "0.5.0"
app.kubernetes.io/version: "edge"
spec:
controllerName: k8s-gateway.nginx.org/nginx-gateway-controller
4 changes: 2 additions & 2 deletions deploy/manifests/service/loadbalancer-aws-nlb.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ metadata:
labels:
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
app.kubernetes.io/version: "0.5.0"
app.kubernetes.io/version: "edge"
annotations:
service.beta.kubernetes.io/aws-load-balancer-type: nlb
spec:
Expand All @@ -17,7 +17,7 @@ spec:
selector:
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
ports:
ports: # Update the following ports to match your Gateway Listener ports
- name: http
port: 80
protocol: TCP
Expand Down
4 changes: 2 additions & 2 deletions deploy/manifests/service/loadbalancer.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ metadata:
labels:
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
app.kubernetes.io/version: "0.5.0"
app.kubernetes.io/version: "edge"
spec:
externalTrafficPolicy: Local
type: LoadBalancer
selector:
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
ports:
ports: # Update the following ports to match your Gateway Listener ports
- name: http
port: 80
protocol: TCP
Expand Down
5 changes: 2 additions & 3 deletions deploy/manifests/service/nodeport.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@ metadata:
labels:
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
app.kubernetes.io/version: "0.5.0"
app.kubernetes.io/version: "edge"
spec:
externalTrafficPolicy: Local
type: NodePort
selector:
app.kubernetes.io/name: nginx-gateway
app.kubernetes.io/instance: nginx-gateway
ports:
ports: # Update the following ports to match your Gateway Listener ports
- name: http
port: 80
protocol: TCP
Expand Down
2 changes: 1 addition & 1 deletion docs/developer/implementing-a-feature.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ practices to ensure a successful feature development process.
11. **Lint code**: See the [run the linter](/docs/developer/quickstart.md#run-the-linter) section of the quickstart
guide for instructions.
12. **Run generators**: See the [Run go generate](/docs/developer/quickstart.md#run-go-generate) and the
[Update NJS module ConfigMaps](/docs/developer/quickstart.md#update-njs-module-configmaps) sections of the
[Update Generated Manifests](/docs/developer/quickstart.md#update-generated-manifests) sections of the
quickstart guide for instructions.
13. **Open pull request**: Open a pull request targeting the `main` branch of
the [nginx-kubernetes-gateway](https://github.com/nginxinc/nginx-kubernetes-gateway/tree/main) repository. The
Expand Down
Loading

0 comments on commit b3dbca2

Please sign in to comment.