Skip to content

feat: add Istio VirtualService Requestmatch to UDS Operator#129

Merged
jeff-mccoy merged 10 commits intomainfrom
operator-vs-enhancement
Jan 26, 2024
Merged

feat: add Istio VirtualService Requestmatch to UDS Operator#129
jeff-mccoy merged 10 commits intomainfrom
operator-vs-enhancement

Conversation

@jeff-mccoy
Copy link
Copy Markdown
Member

@jeff-mccoy jeff-mccoy commented Jan 24, 2024

Description

This PR introduces Istio VirtualService HttpMatchRequest rules for the UDS Operator expose field. While a bit more limited than their Istio counterparts, the following fields are carried over into UDS:

  • ignoreUriCase
  • method
  • name * UDS requires this field (optional in Istio)
  • queryParams
  • uri

Additional this PR mirrors naming validation/generation behavior UDS Operator uses for allow entries, including adding an optional description field to name the generated resource.

Related Issue

Fixes #114

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

@jeff-mccoy jeff-mccoy requested a review from a team as a code owner January 24, 2024 10:30
@rjferguson21
Copy link
Copy Markdown
Contributor

Thanks for doing this so fast @jeff-mccoy.

It might have been a conscious choice but I don't think this implementation will satisfy the GitLab example in the issue where a single service is exposing multiple ports.

For other's context this is what the example VS with matches generates from this PR (notice the single port):

apiVersion: uds.dev/v1alpha1
kind: Package
metadata:
  name: httpbin
  namespace: test-admin-app
spec:
  network:
    expose:
      - service: httpbin
        podLabels:
          app: httpbin
        gateway: admin
        host: demo
        port: 8000
        targetPort: 80
        match:
          - name: test-get-and-prefix
            method:
              # Only allow GET requests
              regex: GET
            uri:
              # Only allow routing to /status/2*, everything else should 404
              prefix: /status/2
          - name: test-exact
            uri:
              # Only allow routing to /status/410
              exact: /status/410
apiVersion: networking.istio.io/v1beta1
kind: VirtualService
metadata:
  name: httpbin-admin-demo
  namespace: test-admin-app
spec:
  gateways:
  - istio-admin-gateway/admin-gateway
  hosts:
  - demo.admin.uds.dev
  http:
  - match:
    - method:
        regex: GET
      name: test-get-and-prefix
      uri:
        prefix: /status/2
    - name: test-exact
      uri:
        exact: /status/410
    route:
    - destination:
        host: httpbin.test-admin-app.svc.cluster.local
        port:
          number: 8000

@jeff-mccoy
Copy link
Copy Markdown
Member Author

Yeah I looked at the example, this should work, but slightly differently (as in 2 expose entries) and dropping the fault entry. I looked at why they were doing that and there are other ways in istio to do that--including using regex in this match to never include the value.

@rjferguson21
Copy link
Copy Markdown
Contributor

That suggestion implies we will create two separate VS? Definitely understand the trade off to keep the Package spec as terse as possible but wanted to clarify that is what you're suggesting as the outcome.

The way the VS are named today it won't let you create multiple VS for a particular pkg/gateway/host combo and overwrites the previous one.

We could potentially merge multiple expose[] into into the same VS...

@jeff-mccoy
Copy link
Copy Markdown
Member Author

Updated & tested with a few various styles to see what it would look like.

Screenshot 2024-01-24 at 7 54 31 PM
Screenshot 2024-01-24 at 7 42 37 PM
Screenshot 2024-01-24 at 7 42 15 PM
Screenshot 2024-01-24 at 7 42 10 PM

@jeff-mccoy jeff-mccoy merged commit a207197 into main Jan 26, 2024
@jeff-mccoy jeff-mccoy deleted the operator-vs-enhancement branch January 26, 2024 23:19
jeff-mccoy pushed a commit that referenced this pull request Jan 27, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.10.0](v0.9.2...v0.10.0)
(2024-01-26)


### Features

* add Istio VirtualService Requestmatch to UDS Operator
([#129](#129))
([a207197](a207197))


### Miscellaneous

* **deps:** update grafana to v10.3.1
([#132](#132))
([09e028c](09e028c))
* **deps:** update istio to v1.20.2
([#75](#75))
([671f977](671f977))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
mjnagel pushed a commit to BagelLab/uds-core that referenced this pull request Nov 14, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.10.0](defenseunicorns/uds-core@v0.9.2...v0.10.0)
(2024-01-26)


### Features

* add Istio VirtualService Requestmatch to UDS Operator
([#129](defenseunicorns#129))
([a207197](defenseunicorns@a207197))


### Miscellaneous

* **deps:** update grafana to v10.3.1
([#132](defenseunicorns#132))
([09e028c](defenseunicorns@09e028c))
* **deps:** update istio to v1.20.2
([#75](defenseunicorns#75))
([671f977](defenseunicorns@671f977))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expanded support for VirtualService generation

4 participants