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

[feature-1091]: Update Authorization Proxy Server Ingress and Use Local Storage for Redis by default #436

Merged
merged 8 commits into from
Jan 31, 2024

Conversation

atye
Copy link
Contributor

@atye atye commented Jan 23, 2024

Description

This updates the Authorization proxy-server ingress to enable connecting by a cluster node IP address instead of requiring one of the hostnames in the ingress, such as csm-authorization.com, because there is extra work to make a hostname like that resolve correctly. It also provisions a local volume for Redis by default if no storage class for Redis is specified.

Changes in some more detail:

Adds v1.10.0 folder for Authorization.

operatorconfig/moduleconfig/authorization/v1.10.0/ingress.yaml contains the following lines to allow connecting by IP address (no host specified).

 - http:
      paths:
      - backend:
          service:
            name: proxy-server
            port:
              number: 8080
        path: /
        pathType: Prefix

Adds operatorconfig/moduleconfig/authorization/v1.10.0/local-provisioner.yaml which contains manifests for a local provisioning storage class and a persistent volume with said storage class. If the REDIS_STORAGE_CLASS env variable is not provided, these resources will be created and used for Redis.

Adds sample file for Authorization v1.10.0 where REDIS_STORAGE_CLASS is not specified by default.

Adds unit test for deploying Authorization with default local storage for Redis.

Adds PROXY_HOST env variable to tests/e2e/env-e2e-test.sh to specify the IP or hostname of the proxy-server for karavictl instead of using hardcoded address that only resolves in a k8s cluster.

Disabled cert-manager dependency in tests/e2e/testfiles/authorization-templates/csm_authorization_proxy_server.yaml because a prerequisite step of Install Authorization Proxy Server is creating a cert which will fail unless cert-manager is installed. So, cert-manager has to be installed before e2e and shouldn't be installed again with Authorization.

Adds e2e scenario for installing Authorization using default, local storage.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
dell/csm#1091

Checklist:

  • I have performed a self-review of my own code to ensure there are no formatting, vetting, linting, or security issues
  • I have verified that new and existing unit tests pass locally with my changes
  • I have not allowed coverage numbers to degenerate
  • I have maintained at least 90% code coverage
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have maintained backward compatibility

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration

New unit test and manual e2e test.

xuluna
xuluna previously approved these changes Jan 30, 2024
@atye atye force-pushed the feature-1091-authorization-ingress-redis branch from 1b26020 to b8f8953 Compare January 31, 2024 19:03
@atye atye merged commit d5efc82 into main Jan 31, 2024
7 of 8 checks passed
@atye atye deleted the feature-1091-authorization-ingress-redis branch January 31, 2024 19:14
ChristianAtDell added a commit that referenced this pull request Oct 15, 2024
…al Storage for Redis by default (#436)

* update ingress, support default redis class, start update e2e

* update auth e2e

* revert new line

* revert new line

* fix lint

* update cr redis comments

* fix e2e

* update proxy_host comment
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.

5 participants