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

Bugfix: Fix multiple configuration environment variables for the storage-users extension #3802

Merged
merged 2 commits into from
May 16, 2022
Merged

Bugfix: Fix multiple configuration environment variables for the storage-users extension #3802

merged 2 commits into from
May 16, 2022

Conversation

wkloucek
Copy link
Contributor

Description

We've fixed multiple environment variable configuration options for the storage-users extension:

  • STORAGE_USERS_GRPC_ADDR was used to configure both the address of the http and grpc server.
    This resulted in a failing startup of the storage-users extension if this config option is set,
    because the service tries to double-bind the configured port (one time for each of the http and grpc server). You can now configure the grpc server's address with the environment variable STORAGE_USERS_GRPC_ADDR and the http server's address with the environment variable STORAGE_USERS_HTTP_ADDR
  • STORAGE_USERS_S3NG_USERS_PROVIDER_ENDPOINT was used to configure the permissions service endpoint for the S3NG driver and was therefore renamed to STORAGE_USERS_S3NG_PERMISSIONS_ENDPOINT
  • It's now possible to configure the permissions service endpoint for all storage drivers with the environment variable STORAGE_USERS_PERMISSION_ENDPOINT, which was previously only used by the S3NG driver.

WARNING: this could be considered a breaking change

Related Issue

Motivation and Context

fix the config

How Has This Been Tested?

  • locally

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@wkloucek wkloucek requested a review from micbar May 16, 2022 06:50
@wkloucek
Copy link
Contributor Author

@micbar is this considered a breaking change? For the STORAGE_USERS_S3NG_USERS_PROVIDER_ENDPOINT change, we could remain backwards compatible. All others are not breaking (STORAGE_USERS_GRPC_ADDR can not be configured by anyone currently because the service refuses to start)

@wkloucek wkloucek mentioned this pull request May 16, 2022
9 tasks
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@mmattel
Copy link
Contributor

mmattel commented May 16, 2022

I guess that this will change the yaml/env file output - therefore docs relevant. Just hooking in so we can trigger a docs build.

Copy link
Contributor

@micbar micbar left a comment

Choose a reason for hiding this comment

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

IMO all could be seen as bugfixes

@wkloucek wkloucek merged commit 61b9b6c into owncloud:master May 16, 2022
@wkloucek wkloucek deleted the fix-storage-users-config branch May 16, 2022 10:46
ownclouders pushed a commit that referenced this pull request May 16, 2022
Merge: 6a7fc50 713bdfc
Author: Willy Kloucek <[email protected]>
Date:   Mon May 16 12:46:31 2022 +0200

    Merge pull request #3802 from wkloucek/fix-storage-users-config

    Bugfix: Fix multiple configuration environment variables for the storage-users extension
@micbar micbar mentioned this pull request May 25, 2022
25 tasks
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