Skip to content

feat: inline auth #288

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.

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

Merged
merged 8 commits into from
Nov 6, 2024
Merged

feat: inline auth #288

merged 8 commits into from
Nov 6, 2024

Conversation

mattwelke
Copy link
Contributor

@mattwelke mattwelke commented Nov 4, 2024

Issue

Resolves #285.

Description

Adds support for inline auth. The order of precedence is:

  1. Implicit (workload identity)
  2. Explicit - k8s secret
  3. Explicit - inline

Example using the new auth style:

apiVersion: validation.spectrocloud.labs/v1alpha1
kind: AzureValidator
metadata:
  name: azurevalidator-communitygalleryimages-one-image-inline-auth
spec:
  auth:
    implicit: false
    credentials:
      tenantId: "<uuid>"
      clientId: "<uuid>"
      clientSecret: "abc123"
      environment: "AzureUSGovernment"
  communityGalleryImageRules:
  - name: rule-1
    gallery:
      location: westus
      name: AKSUbuntu-38d80f77-467a-481f-a8d4-09b6d4220bd2
    images:
    - 1804gen2gpucontainerd
    subscriptionID: 9b16dd0b-1bea-4c9a-a291-65e6f44c4745

Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
@mattwelke mattwelke requested a review from a team as a code owner November 4, 2024 19:37
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. new-feature Net-new feature labels Nov 4, 2024
Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
Copy link
Member

@TylerGillson TylerGillson left a comment

Choose a reason for hiding this comment

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

What about the Azure environment? Can't remember how that's being handled tbh.

Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
@mattwelke
Copy link
Contributor Author

What about the Azure environment? Can't remember how that's being handled tbh.

@TylerGillson Good point. I looked at how it was done before and I don't think it was done well. It only allowed the Azure environment to be specified in the Helm config. That meant that users could only specify it if they were using k8s to evaluate the rules. If they were using direct rule evaluation, they'd be stuck only being able to connect to the public Azure cloud.

I reworked it. Now, the environment to connect to is part of the credentials part of the auth config just like the three main env vars are. It works the same way as them, where if the user specifies them using a k8s secret or inline in the CRD, the env var that the Azure SDK needs gets set.

While reworking it, I changed the order of precedence. Now, k8s secret takes priority over inline config. This made validation code for the values simpler, and it matches the way the other plugins work too, where the secret is used first.

TylerGillson
TylerGillson previously approved these changes Nov 4, 2024
Copy link
Member

@TylerGillson TylerGillson left a comment

Choose a reason for hiding this comment

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

Nice! You manually verified running the controller in k8s as well as running validatorctl in direct mode?

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 4, 2024
Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
@mattwelke
Copy link
Contributor Author

Nice! You manually verified running the controller in k8s as well as running validatorctl in direct mode?

Tested manually in k8s. Did not test in direct mode yet. I'll test that using a replace in validatorctl before merging this.

Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
@TylerGillson TylerGillson self-requested a review November 4, 2024 23:05
TylerGillson
TylerGillson previously approved these changes Nov 4, 2024
@mattwelke
Copy link
Contributor Author

Confirmed working with direct execution. Used the example in the PR desc with real Azure creds.

validatorctl rules check --custom-resources ./crs
Reading plugin specs from file: crs/azure-validator.yaml

                                                                                                                                                                                                                                                                 
                                                                                                                  Executing validator plugin(s)                                                                                                                  
                                                                                                                                                                                                                                                                 


=================
Validation Result
=================

Plugin:           Azure
Name:             validator-plugin-azure-azure-validator
Namespace:        N/A
State:            Succeeded

------------
Rule Results
------------

Validation Rule:        validation-rule-1
Validation Type:        azure-community-gallery-image
Status:                 True
Last Validated:         2024-11-05T12:18:28-05:00
Message:                All required images present in community gallery.

-------
Details
-------
- Found image; Name: '1804gen2gpucontainerd'

Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
Copy link

codecov bot commented Nov 6, 2024

Codecov Report

Attention: Patch coverage is 77.04918% with 14 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/controller/azurevalidator_controller.go 75.86% 9 Missing and 5 partials ⚠️
@@            Coverage Diff             @@
##             main     #288      +/-   ##
==========================================
+ Coverage   60.95%   61.23%   +0.28%     
==========================================
  Files          11       12       +1     
  Lines         945      988      +43     
==========================================
+ Hits          576      605      +29     
- Misses        348      358      +10     
- Partials       21       25       +4     
Files with missing lines Coverage Δ
api/v1alpha1/azurevalidator_types.go 75.00% <ø> (ø)
pkg/utils/strings/strings.go 100.00% <100.00%> (ø)
internal/controller/azurevalidator_controller.go 67.59% <75.86%> (-1.53%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d00fe26...da99247. Read the comment docs.

@mattwelke mattwelke merged commit 92c6d45 into main Nov 6, 2024
8 checks passed
@mattwelke mattwelke deleted the iss285 branch November 6, 2024 02:09
mattwelke pushed a commit that referenced this pull request Nov 6, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.0.22](v0.0.21...v0.0.22)
(2024-11-06)


### Features

* inline auth
([#288](#288))
([92c6d45](92c6d45))


### Other

* make reviewable
([#279](#279))
([17f6bd3](17f6bd3))
* Update renovate.json
([#286](#286))
([d00fe26](d00fe26))


### Dependency Updates

* **deps:** update all non-major dependencies
([#287](#287))
([3ed7f21](3ed7f21))
* **deps:** update golang.org/x/exp digest to 701f63a
([#269](#269))
([ce1c963](ce1c963))
* **deps:** update golang.org/x/exp digest to f66d83c
([#275](#275))
([d2ffcba](d2ffcba))
* **deps:** update kubernetes packages to v0.31.1
([#273](#273))
([90e4333](90e4333))
* **deps:** update kubernetes packages to v0.31.2
([#281](#281))
([be6bbe0](be6bbe0))
* **deps:** update module github.com/azure/azure-sdk-for-go/sdk/azcore
to v1.16.0
([#278](#278))
([1e085cf](1e085cf))
* **deps:** update module
github.com/azure/azure-sdk-for-go/sdk/azidentity to v1.8.0
([#277](#277))
([c4ecbe9](c4ecbe9))
* **deps:** update module github.com/onsi/ginkgo/v2 to v2.21.0
([#283](#283))
([daf6a31](daf6a31))
* **deps:** update module github.com/onsi/gomega to v1.35.0
([#284](#284))
([0cc8916](0cc8916))
* **deps:** update module github.com/validator-labs/validator to v0.1.12
([#280](#280))
([112fa52](112fa52))
* **deps:** update module sigs.k8s.io/cluster-api to v1.8.3
([#270](#270))
([f9619b2](f9619b2))
* **deps:** update module sigs.k8s.io/cluster-api to v1.8.4
([#276](#276))
([bebdff2](bebdff2))
* **deps:** update module sigs.k8s.io/controller-runtime to v0.19.1
([#282](#282))
([ce0bb1f](ce0bb1f))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer new-feature Net-new feature size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🌱 feat: support inline auth
2 participants