Skip to content
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.

Sign up for GitHub

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

feat(ingress): automatic host and path discovery #49

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat(ingress): automatic host and path discovery #49

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cc2fe44
feat(ingress): iterate over hosts and paths
akosveres Nov 4, 2024
b07f082
docs(ingress): Update docs and sample
akosveres Nov 4, 2024
94d2d80
build(trivy): use ECR repository
akosveres Nov 11, 2024
dfad553
build(gha): add smenatic-validate PR step
akosveres Nov 11, 2024
1fdaac6
fix(annotations): handle the resource disable state
akosveres Nov 18, 2024
ccb8805
chore(format): quality of code improvements
akosveres Nov 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .github/workflows/pull-request.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,41 @@ jobs:
uses: aquasecurity/[email protected]
env:
IMAGE: ${{ steps.docker_meta.outputs.tags }}
TRIVY_DB_REPOSITORY: public.ecr.aws/aquasecurity/trivy-db
with:
image-ref: ${{ env.IMAGE }}
format: "table"
exit-code: "1"

semantic-validate:
name: Validate PR title
runs-on: ubuntu-latest
steps:
- uses: amannn/action-semantic-pull-request@v5
id: lint_pr_title
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- uses: marocchino/sticky-pull-request-comment@v2
# When the previous steps fails, the workflow would stop. By adding this
# condition you can continue the execution with the populated error message.
if: always() && (steps.lint_pr_title.outputs.error_message != null)
with:
header: pr-title-lint-error
message: |
Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the [Conventional Commits specification](https://www.conventionalcommits.org/en/v1.0.0/) and it looks like your proposed title needs to be adjusted.

Details:

```
${{ steps.lint_pr_title.outputs.error_message }}
```

# Delete a previous comment when the issue has been resolved
- if: ${{ steps.lint_pr_title.outputs.error_message == null }}
uses: marocchino/sticky-pull-request-comment@v2
with:
header: pr-title-lint-error
delete: true
32 changes: 28 additions & 4 deletions config/samples/network_v1_ingress.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,41 @@ metadata:
name: ingress-sample
annotations:
k8s.checklyhq.com/enabled: "true"
k8s.checklyhq.com/path: "/baz"
# k8s.checklyhq.com/endpoint: "foo.baaz" - Default read from spec.rules[0].host
# k8s.checklyhq.com/success: "200" - Default "200"
# k8s.checklyhq.com/endpoint: "foo.baaz" - Default read from spec.rules[*].host
k8s.checklyhq.com/group: "group-sample"
# k8s.checklyhq.com/muted: "false" # If not set, default "true"
# k8s.checklyhq.com/path: "/baz" - Default read from spec.rules[*].http.paths[*].path
# k8s.checklyhq.com/success: "200" - Default "200"
Comment on lines 7 to +12
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With that it seems we can add only once custom endpoint+path+status. I'd suggest to have one annotation that can define multiple targets. E.g.:

  annotations:
    k8s.checklyhq.com/api_monitor: "200|https://foo.baaz/baz,301|http://redirect.demo"

Even better, maybe include JSON in the value that is directly tied to API.

JSON for flexibility, simple syntax for UX. Should be decided, but I'd move from ability to set only one endpoint.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100% agree that we need something, not sure what the best path forward is, I'd hate to manage JSON inside YAML..

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we need to see if we can find best simple abstraction of what we want to achieve here and not be slapped in the face because it does not support some basic functionality. I'd still have API reference in mind here to make sure we cover basics.

What do you propose?

Also, what would be the minimum (original) API JSON request for hostname+path? Just to see verbosity. Benefit here is the ability to support those kind of annotations even in Service resources and documentation is already available.

spec:
rules:
- host: "foo.bar"
http:
paths:
- path: /
- path: /foo
pathType: ImplementationSpecific
backend:
service:
name: test-service
port:
number: 8080
- path: /bar
pathType: ImplementationSpecific
backend:
service:
name: test-service
port:
number: 8080
- host: "example.com"
http:
paths:
- path: /tea
pathType: ImplementationSpecific
backend:
service:
name: test-service
port:
number: 8080
- path: /coffee
pathType: ImplementationSpecific
backend:
service:
Expand Down
27 changes: 20 additions & 7 deletions docs/ingress.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
# ingress

We also support kubernetes native `ingress` resources. See [official docs](https://kubernetes.io/docs/concepts/services-networking/ingress/) for more details on what they are and what they do.
Support for kubernetes native `ingress` resources. See [official docs](https://kubernetes.io/docs/concepts/services-networking/ingress/) for more details on what they are and what they do.

We pull out information with the use of `annotations`. The information from the annotations is used to create `ApiCheck` resources, we make use of [ownerReferences](https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/) to link ingress resources to ApiCheck resources.
We pull out information with the use of `annotations` and use the built in spec. The information from the annotations is used to create `ApiCheck` resources, we make use of [ownerReferences](https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/) to link ingress resources to ApiCheck resources.

> ***Warning***
> We currently only support one API check / ingress resource.
> We currently only support API checks for ingress resources.

## Logic of discovery

We iterate over the ingress resource's specifications to work out what needs to be created. The operator creates one ApiCheck resource for each `host` + `path`, if in your ingress resource you have 2 hosts with 3 paths each, you'll end up with 6 ApiChecks created.

Specific annotations are optional, as we can't automatically discover the group you want the Checkly APIChecks to be deployd in.

## Configuration options

Expand All @@ -14,10 +20,10 @@ The name of the API Check derives from the `metadata.name` of the `ingress` reso
| Annotation | Details | Default |
|--------------------|-------------|---------|
| `k8s.checklyhq.com/enabled` | Bool; Should the operator read the annotations or not | `false` (*required) |
| `k8s.checklyhq.com/path` | String; The URI to put after the `endpoint`, for example `/path` | "" (*required) |
| `k8s.checklyhq.com/endpoint` | String; The host of the URL, for example `/` | Value of `spec.rules[0].Host`, defaults to `https://` (*required) |
| `k8s.checklyhq.com/endpoint` | String; The host of the URL, for example `/` | Value of `spec.rules[0].Host`, defaults to `https://` |
| `k8s.checklyhq.com/group` | String; Name of the group to which the check belongs; Kubernetes `Group` resource name` | none (*required)|
| `k8s.checklyhq.com/muted` | String; Is the check muted or not | `true` |
| `k8s.checklyhq.com/path` | String; The URI to put after the `endpoint`, for example `/path` | ""|
| `k8s.checklyhq.com/success` | String; The expected success code | `200` |

### Example
Expand All @@ -29,7 +35,7 @@ metadata:
name: checkly-operator-ingress
annotations:
k8s.checklyhq.com/enabled: "true"
k8s.checklyhq.com/path: "/baz"
# k8s.checklyhq.com/path: "/baz" - Default read from spec.rules[0].http.paths[*].path
# k8s.checklyhq.com/endpoint: "foo.baaz" - Default read from spec.rules[0].host
# k8s.checklyhq.com/success: "200" - Default "200"
k8s.checklyhq.com/group: "group-sample"
Expand All @@ -39,7 +45,14 @@ spec:
- host: "foo.bar"
http:
paths:
- path: /
- path: /foo
pathType: ImplementationSpecific
backend:
service:
name: test-service
port:
number: 8080
- path: /bar
pathType: ImplementationSpecific
backend:
service:
Expand Down
Loading
Loading