From 6cd255d516b1981ffb3bcae6434c55f09e30b0a0 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 28 Jan 2021 11:44:48 -0800 Subject: [PATCH] fix: allow updated domain names in federation (#11365) additionally also disallow overlapping domain names --- cmd/bucket-handlers.go | 47 ++++++++++++++++++------ cmd/common-main.go | 9 +++++ cmd/config/dns/etcd_dns.go | 41 +++++++++++---------- cmd/config/errors.go | 6 ++++ cmd/utils.go | 73 ++++++++++++++++++++++++-------------- cmd/utils_test.go | 2 +- cmd/web-handlers.go | 4 ++- 7 files changed, 123 insertions(+), 59 deletions(-) diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index 562386b9b..a1b00605b 100644 --- a/cmd/bucket-handlers.go +++ b/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, - // 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)) - } + wg.Add(1) + go func(bucket string) { + defer wg.Done() + // We go to here, so we know the bucket no longer exists, + // 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 diff --git a/cmd/common-main.go b/cmd/common-main.go index 30cd5c60f..b7bba09c2 100644 --- a/cmd/common-main.go +++ b/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, "") diff --git a/cmd/config/dns/etcd_dns.go b/cmd/config/dns/etcd_dns.go index cc1664297..09a338113 100644 --- a/cmd/config/dns/etcd_dns.go +++ b/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) - if _, err := c.etcdClient.Delete(dctx, key+etcdPathSeparator+record.Host); err != nil { - dcancel() + ctx, cancel := context.WithTimeout(context.Background(), defaultContextTimeout) + _, err := c.etcdClient.Delete(ctx, key+etcdPathSeparator+record.Host) + cancel() + if err != nil { return err } - dcancel() } return nil } diff --git a/cmd/config/errors.go b/cmd/config/errors.go index 10dba2081..67690a074 100644 --- a/cmd/config/errors.go +++ b/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", diff --git a/cmd/utils.go b/cmd/utils.go index f154ee0a0..40cc4c39d 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -665,37 +665,56 @@ func restQueries(keys ...string) []string { return accumulator } -// lcp finds the longest common prefix of the input strings. -// It compares by bytes instead of runes (Unicode code points). -// It's up to the caller to do Unicode normalization if desired -// (e.g. see golang.org/x/text/unicode/norm). -func lcp(l []string) string { - // Special cases first - switch len(l) { - case 0: +// Suffix returns the longest common suffix of the provided strings +func lcpSuffix(strs []string) string { + return lcp(strs, false) +} + +func lcp(strs []string, pre bool) string { + // short-circuit empty list + if len(strs) == 0 { return "" - case 1: - return l[0] } - // LCP of min and max (lexigraphically) - // is the LCP of the whole set. - min, max := l[0], l[0] - for _, s := range l[1:] { - switch { - case s < min: - min = s - case s > max: - max = s + xfix := strs[0] + // short-circuit single-element list + if len(strs) == 1 { + return xfix + } + // compare first to rest + for _, str := range strs[1:] { + xfixl := len(xfix) + 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++ { - 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 + return xfix } // Returns the mode in which MinIO is running diff --git a/cmd/utils_test.go b/cmd/utils_test.go index 90e329b86..4aaf44b2f 100644 --- a/cmd/utils_test.go +++ b/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) } diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go index ec1c85653..10793821e 100644 --- a/cmd/web-handlers.go +++ b/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 {