fix: allow updated domain names in federation (#11365)

additionally also disallow overlapping domain names

cmd/bucket-handlers.go

@@ -25,8 +25,10 @@ import (
 	"net/url"
 	"path"
 	"path/filepath"
+	"sort"
 	"strconv"
 	"strings"
+	"sync"
 
 	"github.com/google/uuid"
 	"github.com/gorilla/mux"
@@ -72,7 +74,7 @@ func initFederatorBackend(buckets []BucketInfo, objLayer ObjectLayer) {
 
 	// Get buckets in the DNS
 	dnsBuckets, err := globalDNSConfig.List()
-	if err != nil && err != dns.ErrNoEntriesFound && err != dns.ErrNotImplemented {
+	if err != nil && !IsErrIgnored(err, dns.ErrNoEntriesFound, dns.ErrNotImplemented, dns.ErrDomainMissing) {
 		logger.LogIf(GlobalContext, err)
 		return
 	}
@@ -80,6 +82,10 @@ func initFederatorBackend(buckets []BucketInfo, objLayer ObjectLayer) {
 	bucketsSet := set.NewStringSet()
 	bucketsToBeUpdated := set.NewStringSet()
 	bucketsInConflict := set.NewStringSet()
+
+	// This means that domain is updated, we should update
+	// all bucket entries with new domain name.
+	domainMissing := err == dns.ErrDomainMissing
 	if dnsBuckets != nil {
 		for _, bucket := range buckets {
 			bucketsSet.Add(bucket.Name)
@@ -89,11 +95,16 @@ func initFederatorBackend(buckets []BucketInfo, objLayer ObjectLayer) {
 				continue
 			}
 			if !globalDomainIPs.Intersection(set.CreateStringSet(getHostsSlice(r)...)).IsEmpty() {
-				if globalDomainIPs.Difference(set.CreateStringSet(getHostsSlice(r)...)).IsEmpty() {
+				if globalDomainIPs.Difference(set.CreateStringSet(getHostsSlice(r)...)).IsEmpty() && !domainMissing {
 					// No difference in terms of domainIPs and nothing
 					// has changed so we don't change anything on the etcd.
+					//
+					// Additionally also check if domain is updated/missing with more
+					// entries, if that is the case we should update the
+					// new domain entries as well.
 					continue
 				}
+
 				// if domain IPs intersect then it won't be an empty set.
 				// such an intersection means that bucket exists on etcd.
 				// but if we do see a difference with local domain IPs with
@@ -102,6 +113,7 @@ func initFederatorBackend(buckets []BucketInfo, objLayer ObjectLayer) {
 				bucketsToBeUpdated.Add(bucket.Name)
 				continue
 			}
+
 			// No IPs seem to intersect, this means that bucket exists but has
 			// different IP addresses perhaps from a different deployment.
 			// bucket names are globally unique in federation at a given
@@ -112,8 +124,8 @@ func initFederatorBackend(buckets []BucketInfo, objLayer ObjectLayer) {
 	}
 
 	// Add/update buckets that are not registered with the DNS
-	g := errgroup.WithNErrs(len(buckets))
 	bucketsToBeUpdatedSlice := bucketsToBeUpdated.ToSlice()
+	g := errgroup.WithNErrs(len(bucketsToBeUpdatedSlice))
 	for index := range bucketsToBeUpdatedSlice {
 		index := index
 		g.Go(func() error {
@@ -128,9 +140,10 @@ func initFederatorBackend(buckets []BucketInfo, objLayer ObjectLayer) {
 	}
 
 	for _, bucket := range bucketsInConflict.ToSlice() {
-		logger.LogIf(GlobalContext, fmt.Errorf("Unable to add bucket DNS entry for bucket %s, an entry exists for the same bucket. Use one of these IP addresses %v to access the bucket", bucket, globalDomainIPs.ToSlice()))
+		logger.LogIf(GlobalContext, fmt.Errorf("Unable to add bucket DNS entry for bucket %s, an entry exists for the same bucket by a different tenant. This local bucket will be ignored. Bucket names are globally unique in federated deployments. Use path style requests on following addresses '%v' to access this bucket.", bucket, globalDomainIPs.ToSlice()))
 	}
 
+	var wg sync.WaitGroup
 	// Remove buckets that are in DNS for this server, but aren't local
 	for bucket, records := range dnsBuckets {
 		if bucketsSet.Contains(bucket) {
@@ -142,13 +155,18 @@ func initFederatorBackend(buckets []BucketInfo, objLayer ObjectLayer) {
 			continue
 		}
 
-		// We go to here, so we know the bucket no longer exists,
+		wg.Add(1)
-		// but is registered in DNS to this server
+		go func(bucket string) {
-		if err = globalDNSConfig.Delete(bucket); err != nil {
+			defer wg.Done()
-			logger.LogIf(GlobalContext, fmt.Errorf("Failed to remove DNS entry for %s due to %w",
+			// We go to here, so we know the bucket no longer exists,
-				bucket, err))
+			// but is registered in DNS to this server
-		}
+			if err := globalDNSConfig.Delete(bucket); err != nil {
+				logger.LogIf(GlobalContext, fmt.Errorf("Failed to remove DNS entry for %s due to %w",
+					bucket, err))
+			}
+		}(bucket)
 	}
+	wg.Wait()
 }
 
 // GetBucketLocationHandler - GET Bucket location.
@@ -280,7 +298,9 @@ func (api objectAPIHandlers) ListBucketsHandler(w http.ResponseWriter, r *http.R
 	var bucketsInfo []BucketInfo
 	if globalDNSConfig != nil && globalBucketFederation {
 		dnsBuckets, err := globalDNSConfig.List()
-		if err != nil && err != dns.ErrNoEntriesFound {
+		if err != nil && !IsErrIgnored(err,
+			dns.ErrNoEntriesFound,
+			dns.ErrDomainMissing) {
 			writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
 			return
 		}
@@ -290,6 +310,11 @@ func (api objectAPIHandlers) ListBucketsHandler(w http.ResponseWriter, r *http.R
 				Created: dnsRecords[0].CreationDate,
 			})
 		}
+
+		sort.Slice(bucketsInfo, func(i, j int) bool {
+			return bucketsInfo[i].Name < bucketsInfo[j].Name
+		})
+
 	} else {
 		// Invoke the list buckets.
 		var err error

cmd/common-main.go

@@ -28,6 +28,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"sort"
 	"strings"
 	"time"
 
@@ -254,6 +255,14 @@ func handleCommonEnvVars() {
 			}
 			globalDomainNames = append(globalDomainNames, domainName)
 		}
+		sort.Strings(globalDomainNames)
+		lcpSuf := lcpSuffix(globalDomainNames)
+		for _, domainName := range globalDomainNames {
+			if domainName == lcpSuf {
+				logger.Fatal(config.ErrOverlappingDomainValue(nil).Msg("Overlapping domains `%s` not allowed", globalDomainNames),
+					"Invalid MINIO_DOMAIN value in environment variable")
+			}
+		}
 	}
 
 	publicIPs := env.Get(config.EnvPublicIPs, "")

cmd/config/dns/etcd_dns.go

@@ -34,6 +34,9 @@ import (
 // ErrNoEntriesFound - Indicates no entries were found for the given key (directory)
 var ErrNoEntriesFound = errors.New("No entries found for this key")
 
+// ErrDomainMissing - Indicates domain is missing
+var ErrDomainMissing = errors.New("domain is missing")
+
 const etcdPathSeparator = "/"
 
 // create a new coredns service record for the bucket.
@@ -57,9 +60,9 @@ func (c *CoreDNS) List() (map[string][]SrvRecord, error) {
 	var srvRecords = map[string][]SrvRecord{}
 	for _, domainName := range c.domainNames {
 		key := msg.Path(fmt.Sprintf("%s.", domainName), c.prefixPath)
-		records, err := c.list(key)
+		records, err := c.list(key+etcdPathSeparator, true)
 		if err != nil {
-			return nil, err
+			return srvRecords, err
 		}
 		for _, record := range records {
 			if record.Key == "" {
@@ -76,7 +79,7 @@ func (c *CoreDNS) Get(bucket string) ([]SrvRecord, error) {
 	var srvRecords []SrvRecord
 	for _, domainName := range c.domainNames {
 		key := msg.Path(fmt.Sprintf("%s.%s.", bucket, domainName), c.prefixPath)
-		records, err := c.list(key)
+		records, err := c.list(key+etcdPathSeparator, false)
 		if err != nil {
 			return nil, err
 		}
@@ -109,19 +112,25 @@ func msgUnPath(s string) string {
 
 // Retrieves list of entries under the key passed.
 // Note that this method fetches entries upto only two levels deep.
-func (c *CoreDNS) list(key string) ([]SrvRecord, error) {
+func (c *CoreDNS) list(key string, domain bool) ([]SrvRecord, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), defaultContextTimeout)
 	r, err := c.etcdClient.Get(ctx, key, clientv3.WithPrefix())
 	defer cancel()
 	if err != nil {
 		return nil, err
 	}
+
 	if r.Count == 0 {
 		key = strings.TrimSuffix(key, etcdPathSeparator)
 		r, err = c.etcdClient.Get(ctx, key)
 		if err != nil {
 			return nil, err
 		}
+		// only if we are looking at `domain` as true
+		// we should return error here.
+		if domain && r.Count == 0 {
+			return nil, ErrDomainMissing
+		}
 	}
 
 	var srvRecords []SrvRecord
@@ -166,11 +175,11 @@ func (c *CoreDNS) Put(bucket string) error {
 			key = key + etcdPathSeparator + ip
 			ctx, cancel := context.WithTimeout(context.Background(), defaultContextTimeout)
 			_, err = c.etcdClient.Put(ctx, key, string(bucketMsg))
-			defer cancel()
+			cancel()
 			if err != nil {
 				ctx, cancel = context.WithTimeout(context.Background(), defaultContextTimeout)
 				c.etcdClient.Delete(ctx, key)
-				defer cancel()
+				cancel()
 				return err
 			}
 		}
@@ -182,18 +191,12 @@ func (c *CoreDNS) Put(bucket string) error {
 func (c *CoreDNS) Delete(bucket string) error {
 	for _, domainName := range c.domainNames {
 		key := msg.Path(fmt.Sprintf("%s.%s.", bucket, domainName), c.prefixPath)
-		srvRecords, err := c.list(key)
+		ctx, cancel := context.WithTimeout(context.Background(), defaultContextTimeout)
+		_, err := c.etcdClient.Delete(ctx, key+etcdPathSeparator, clientv3.WithPrefix())
+		cancel()
 		if err != nil {
 			return err
 		}
-		for _, record := range srvRecords {
-			dctx, dcancel := context.WithTimeout(context.Background(), defaultContextTimeout)
-			if _, err = c.etcdClient.Delete(dctx, key+etcdPathSeparator+record.Host); err != nil {
-				dcancel()
-				return err
-			}
-			dcancel()
-		}
 	}
 	return nil
 }
@@ -203,12 +206,12 @@ func (c *CoreDNS) DeleteRecord(record SrvRecord) error {
 	for _, domainName := range c.domainNames {
 		key := msg.Path(fmt.Sprintf("%s.%s.", record.Key, domainName), c.prefixPath)
 
-		dctx, dcancel := context.WithTimeout(context.Background(), defaultContextTimeout)
+		ctx, cancel := context.WithTimeout(context.Background(), defaultContextTimeout)
-		if _, err := c.etcdClient.Delete(dctx, key+etcdPathSeparator+record.Host); err != nil {
+		_, err := c.etcdClient.Delete(ctx, key+etcdPathSeparator+record.Host)
-			dcancel()
+		cancel()
+		if err != nil {
 			return err
 		}
-		dcancel()
 	}
 	return nil
 }

cmd/config/errors.go

@@ -30,6 +30,12 @@ var (
 		"Can only accept `on` and `off` values. To enable O_SYNC for fs backend, set this value to `on`",
 	)
 
+	ErrOverlappingDomainValue = newErrFn(
+		"Overlapping domain values",
+		"Please check the passed value",
+		"MINIO_DOMAIN only accepts non-overlapping domain values",
+	)
+
 	ErrInvalidDomainValue = newErrFn(
 		"Invalid domain value",
 		"Please check the passed value",

cmd/utils.go

@@ -665,37 +665,56 @@ func restQueries(keys ...string) []string {
 	return accumulator
 }
 
-// lcp finds the longest common prefix of the input strings.
+// Suffix returns the longest common suffix of the provided strings
-// It compares by bytes instead of runes (Unicode code points).
+func lcpSuffix(strs []string) string {
-// It's up to the caller to do Unicode normalization if desired
+	return lcp(strs, false)
-// (e.g. see golang.org/x/text/unicode/norm).
+}
-func lcp(l []string) string {
+
-	// Special cases first
+func lcp(strs []string, pre bool) string {
-	switch len(l) {
+	// short-circuit empty list
-	case 0:
+	if len(strs) == 0 {
 		return ""
-	case 1:
-		return l[0]
 	}
-	// LCP of min and max (lexigraphically)
+	xfix := strs[0]
-	// is the LCP of the whole set.
+	// short-circuit single-element list
-	min, max := l[0], l[0]
+	if len(strs) == 1 {
-	for _, s := range l[1:] {
+		return xfix
-		switch {
+	}
-		case s < min:
+	// compare first to rest
-			min = s
+	for _, str := range strs[1:] {
-		case s > max:
+		xfixl := len(xfix)
-			max = s
+		strl := len(str)
+		// short-circuit empty strings
+		if xfixl == 0 || strl == 0 {
+			return ""
+		}
+		// maximum possible length
+		maxl := xfixl
+		if strl < maxl {
+			maxl = strl
+		}
+		// compare letters
+		if pre {
+			// prefix, iterate left to right
+			for i := 0; i < maxl; i++ {
+				if xfix[i] != str[i] {
+					xfix = xfix[:i]
+					break
+				}
+			}
+		} else {
+			// suffix, iterate right to left
+			for i := 0; i < maxl; i++ {
+				xi := xfixl - i - 1
+				si := strl - i - 1
+				if xfix[xi] != str[si] {
+					xfix = xfix[xi+1:]
+					break
+				}
+			}
 		}
 	}
-	for i := 0; i < len(min) && i < len(max); i++ {
+	return xfix
-		if min[i] != max[i] {
-			return min[:i]
-		}
-	}
-	// In the case where lengths are not equal but all bytes
-	// are equal, min is the answer ("foo" < "foobar").
-	return min
 }
 
 // Returns the mode in which MinIO is running

cmd/utils_test.go

@@ -460,7 +460,7 @@ func TestLCP(t *testing.T) {
 	}
 
 	for i, test := range testCases {
-		foundPrefix := lcp(test.prefixes)
+		foundPrefix := lcp(test.prefixes, true)
 		if foundPrefix != test.commonPrefix {
 			t.Fatalf("Test %d: Common prefix found: `%v`, expected: `%v`", i+1, foundPrefix, test.commonPrefix)
 		}

cmd/web-handlers.go

@@ -354,7 +354,9 @@ func (web *webAPIHandlers) ListBuckets(r *http.Request, args *WebGenericArgs, re
 	// If etcd, dns federation configured list buckets from etcd.
 	if globalDNSConfig != nil && globalBucketFederation {
 		dnsBuckets, err := globalDNSConfig.List()
-		if err != nil && err != dns.ErrNoEntriesFound {
+		if err != nil && !IsErrIgnored(err,
+			dns.ErrNoEntriesFound,
+			dns.ErrDomainMissing) {
 			return toJSONError(ctx, err)
 		}
 		for _, dnsRecords := range dnsBuckets {