spring-attic / spring-cloud-aws

All development has moved to https://github.com/awspring/spring-cloud-aws Integration for Amazon Web Services APIs with Spring
https://awspring.io/
Apache License 2.0
589 stars 376 forks source link

Add ability for SimpleStorageResource.getURL() to account for custom repositories not hosted in AWS #774

Closed celdridge91190 closed 2 years ago

celdridge91190 commented 3 years ago

Type: Feature

Is your feature request related to a problem? Please describe. When trying to use Spring Cloud config with an S3 backend it is required to add spring-cloud-aws as a dependency to be able to take advantage of the SimpleStorageProtocolResolver in order to serve plain text configuration files. The Config server does a check to ensure the plain text file is available at the specified location which calls on the getURL method in SimpleStorageResource When using a custom S3 repository, such as a local server or ECS repository this method fails when calling on getRegion on the AmazonS3Client as it expects the URL to be in the standard AWS format.

Describe the solution you'd like The getURL() method in SimpleStorageResource method should check the endpoint to see if it is a custom endpoint and build the https URL using the hostname included with the endpoint property instead.

Describe alternatives you've considered I was able to workaround this temporarily by including a custom ResourceRepository that skips the PathUtils.checkResource check within Spring Cloud config to get around this. The issue with this is that I need to copy almost all of the code from GenericResourceRepository in order to get this to work which means I need to track changes to that file and copy them into my custom file if they're needed with future releases of Spring Cloud Config. Code below

package com.paychex.cloud.config.configuration.s3;

import org.springframework.cloud.config.server.environment.SearchPathLocator;
import org.springframework.cloud.config.server.resource.NoSuchResourceException;
import org.springframework.cloud.config.server.resource.ResourceRepository;
import org.springframework.cloud.config.server.support.PathUtils;
import org.springframework.context.ResourceLoaderAware;
import org.springframework.core.io.Resource;
import org.springframework.core.io.ResourceLoader;
import org.springframework.util.StringUtils;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedHashSet;
import java.util.Set;

public class S3ResourceRepository implements ResourceRepository, ResourceLoaderAware {

    private SearchPathLocator service;
    public S3ResourceRepository(SearchPathLocator service) {
        this.service = service;
    }
    private ResourceLoader resourceLoader;

    @Override
    public void setResourceLoader(ResourceLoader resourceLoader) {
        this.resourceLoader = resourceLoader;
    }

    @Override
    public synchronized Resource findOne(String application, String profile, String label,
                                         String path) {

        if (StringUtils.hasText(path)) {
            String[] locations = this.service.getLocations(application, profile, label)
                    .getLocations();
            ArrayList<Resource> locationResources = new ArrayList<>();
            for (String location : locations) {
                if (!PathUtils.isInvalidEncodedLocation(location)) {
                    locationResources.add(this.resourceLoader.getResource(location));
                }
            }

            try {
                for (Resource location : locationResources) {
                    for (String local : getProfilePaths(profile, path)) {
                        if (!PathUtils.isInvalidPath(local)
                                && !PathUtils.isInvalidEncodedPath(local)) {
                            Resource file = location.createRelative(local);
                            if (file.exists() && file.isReadable()) {
                                return file;
                            }
                        }
                    }
                }
            }
            catch (IOException e) {
                throw new NoSuchResourceException(
                        "Error : " + path + ". (" + e.getMessage() + ")");
            }
        }
        throw new NoSuchResourceException("Not found: " + path);
    }

    private Collection<String> getProfilePaths(String profiles, String path) {
        Set<String> paths = new LinkedHashSet<>();
        for (String profile : StringUtils.commaDelimitedListToSet(profiles)) {
            if (!StringUtils.hasText(profile) || "default".equals(profile)) {
                paths.add(path);
            }
            else {
                String ext = StringUtils.getFilenameExtension(path);
                String file = path;
                if (ext != null) {
                    ext = "." + ext;
                    file = StringUtils.stripFilenameExtension(path);
                }
                else {
                    ext = "";
                }
                paths.add(file + "-" + profile + ext);
            }
        }
        paths.add(path);
        return paths;
    }

}

Another workaround considered was to request a change to the SDK getRegion method but the developers have stated that the functionality is intentional and changes to it would be breaking for the framework as well as consumers of the framework that depend on it's functionality https://github.com/aws/aws-sdk-java/issues/2281

Additional context Add any other context or screenshots about the feature request here.

maciejwalkowiak commented 2 years ago

@celdridge91190 issue moved to https://github.com/awspring/spring-cloud-aws/issues/166. Contributions welcome!