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.

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

[ASM-13755] Fix allowedAccessPermissions Type in Gateway Charts #280

Merged
merged 13 commits into from
Feb 13, 2025

Conversation

kgal-akl
Copy link
Contributor

@kgal-akl kgal-akl commented Jan 29, 2025

Description

When deploying a Gateway using either akeyless-api-gateway or akeyless-gateway and filling out the akeylessUserAuth.allowedAccessPermissions property in values.yaml, the comment above the property describes it as a list. However, the default value is an object:

# Configure this list to add one or more allowed access to this GW, with the specified permissions and sub-claims.
# Name must be unique.
# Empty permissions will implicitly give the access admin permission.
allowedAccessPermissions: { }

When filling this section with a list of access permissions:

allowedAccessPermissions:
    - name: kgal-access-permission-1
      access_id: p-1234
    - name: kgal-access-permission-2
      access_id: p-1235

And installing it, we get a warning:

helm upgrade akeyless-gw akeyless/akeyless-api-gateway -f values.yaml --install 

coalesce.go:286: warning: cannot overwrite table with non table for akeyless-api-gateway.akeylessUserAuth.allowedAccessPermissions (map[])

Solution

To remove this warning, this PR introduces a few changes:

  1. The values schema was changed to match the expected type and structure of the allowedAccessPermissions array in the API gateway chart.
  2. The default value in the values.yaml was change to a list.
  3. Add values.schema.json to unified gateway chart.

Testing

To test this change, I used the same chart that produced the issue and used the changes in this ref to rerun the installation. As we can see in the output below, the warning is gone:

helm repo add akeyless-dev git+https://github.com/akeylesslabs/helm-charts@charts?ref=fix-allowedAccessPermissions-type-api-gw
"akeyless-dev" has been added to your repositories

helm upgrade akeyless-gw akeyless-dev/akeyless-api-gateway -f values.yaml --install

Release "akeyless-gw" has been upgraded. Happy Helming!
NAME: akeyless-gw
LAST DEPLOYED: Wed Jan 29 17:46:31 2025
NAMESPACE: default
STATUS: deployed
REVISION: 3
TEST SUITE: None

@kgal-akl kgal-akl requested review from shaym007 and a team January 29, 2025 18:37
@kgal-akl kgal-akl changed the title Fix allowedAccessPermissions Type in API Gateway Chart [ASM-13755] Fix allowedAccessPermissions Type in API Gateway Chart Feb 3, 2025
@kgal-akl
Copy link
Contributor Author

kgal-akl commented Feb 4, 2025

@OriBenHur-akeyless - As this issue is also relevant for the akeyless-gateway chart but this chart, unlike akeyless-api-gateway doesn’t include a values.schema.json file, should I add a values.schema.json to akeyless-gateway chart as part of this PR?

@OriBenHur-akeyless
Copy link
Contributor

It should have a schema also

@kgal-akl
Copy link
Contributor Author

kgal-akl commented Feb 7, 2025

@michelsk @OriBenHur-akeyless - schema added to unified gw as well. Let me know if I can merge.

@kgal-akl kgal-akl changed the title [ASM-13755] Fix allowedAccessPermissions Type in API Gateway Chart [ASM-13755] Fix allowedAccessPermissions Type in Gateway Charts Feb 12, 2025
@kgal-akl kgal-akl merged commit cf989e4 into main Feb 13, 2025
1 check passed
@kgal-akl kgal-akl deleted the fix-allowedAccessPermissions-type-api-gw branch February 13, 2025 08:14
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.

3 participants