(iceberg-go) branch main updated: feat(catalog): Make rest catalog auth scope configurable (#265)

[zeroshade]

This is an automated email from the ASF dual-hosted git repository.

zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git


The following commit(s) were added to refs/heads/main by this push:
     new e685d65  feat(catalog): Make rest catalog auth scope configurable 
(#265)
e685d65 is described below

commit e685d654d5abb4d4d9bce82b652c0ece868f767d
Author: Rick Curtis <curti...@gmail.com>
AuthorDate: Wed Jan 22 12:53:02 2025 -0600

    feat(catalog): Make rest catalog auth scope configurable (#265)
    
    fixes https://github.com/apache/iceberg-go/issues/263
    
    As part of debugging the test update I noticed that multiple http
    clients are created each time a rest client is created
---
 catalog/catalog.go   |  7 +++++++
 catalog/rest.go      | 24 +++++++++++-------------
 catalog/rest_test.go |  6 ++++--
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/catalog/catalog.go b/catalog/catalog.go
index c611dd4..bb843b1 100644
--- a/catalog/catalog.go
+++ b/catalog/catalog.go
@@ -128,6 +128,12 @@ func WithPrefix(prefix string) Option[RestCatalog] {
        }
 }
 
+func WithScope(scope string) Option[RestCatalog] {
+       return func(o *options) {
+               o.scope = scope
+       }
+}
+
 type Option[T GlueCatalog | RestCatalog] func(*options)
 
 type options struct {
@@ -144,6 +150,7 @@ type options struct {
        sigv4Service      string
        prefix            string
        authUri           *url.URL
+       scope             string
 }
 
 type PropertiesUpdateSummary struct {
diff --git a/catalog/rest.go b/catalog/rest.go
index a38c088..636d04e 100644
--- a/catalog/rest.go
+++ b/catalog/rest.go
@@ -468,16 +468,10 @@ func (r *RestCatalog) init(ops *options, uri string) 
error {
        }
 
        r.baseURI = baseuri.JoinPath("v1")
-       if ops, err = r.fetchConfig(ops); err != nil {
+       if r.cl, ops, err = r.fetchConfig(ops); err != nil {
                return err
        }
 
-       cl, err := r.createSession(ops)
-       if err != nil {
-               return err
-       }
-
-       r.cl = cl
        if ops.prefix != "" {
                r.baseURI = r.baseURI.JoinPath(ops.prefix)
        }
@@ -491,11 +485,15 @@ func (r *RestCatalog) fetchAccessToken(cl *http.Client, 
creds string, opts *opti
                clientID, clientSecret = "", clientID
        }
 
+       scope := "catalog"
+       if opts.scope != "" {
+               scope = opts.scope
+       }
        data := url.Values{
                "grant_type":    {"client_credentials"},
                "client_id":     {clientID},
                "client_secret": {clientSecret},
-               "scope":         {"catalog"},
+               "scope":         {scope},
        }
 
        uri := opts.authUri
@@ -575,7 +573,7 @@ func (r *RestCatalog) createSession(opts *options) 
(*http.Client, error) {
        return cl, nil
 }
 
-func (r *RestCatalog) fetchConfig(opts *options) (*options, error) {
+func (r *RestCatalog) fetchConfig(opts *options) (*http.Client, *options, 
error) {
        params := url.Values{}
        if opts.warehouseLocation != "" {
                params.Set(keyWarehouseLocation, opts.warehouseLocation)
@@ -586,12 +584,12 @@ func (r *RestCatalog) fetchConfig(opts *options) 
(*options, error) {
 
        sess, err := r.createSession(opts)
        if err != nil {
-               return nil, err
+               return nil, nil, err
        }
 
        rsp, err := doGet[configResponse](context.Background(), route, 
[]string{}, sess, nil)
        if err != nil {
-               return nil, err
+               return nil, nil, err
        }
 
        cfg := rsp.Defaults
@@ -608,12 +606,12 @@ func (r *RestCatalog) fetchConfig(opts *options) 
(*options, error) {
        if uri, ok := cfg["uri"]; ok {
                r.baseURI, err = url.Parse(uri)
                if err != nil {
-                       return nil, err
+                       return nil, nil, err
                }
                r.baseURI = r.baseURI.JoinPath("v1")
        }
 
-       return o, nil
+       return sess, o, nil
 }
 
 func (r *RestCatalog) Name() string             { return r.name }
diff --git a/catalog/rest_test.go b/catalog/rest_test.go
index 479fd70..2d0302c 100644
--- a/catalog/rest_test.go
+++ b/catalog/rest_test.go
@@ -83,6 +83,7 @@ func (r *RestCatalogSuite) TearDownTest() {
 }
 
 func (r *RestCatalogSuite) TestToken200() {
+       const scope = "myscope"
        r.mux.HandleFunc("/v1/oauth/tokens", func(w http.ResponseWriter, req 
*http.Request) {
                r.Equal(http.MethodPost, req.Method)
 
@@ -93,7 +94,7 @@ func (r *RestCatalogSuite) TestToken200() {
                r.Equal(values.Get("grant_type"), "client_credentials")
                r.Equal(values.Get("client_id"), "client")
                r.Equal(values.Get("client_secret"), "secret")
-               r.Equal(values.Get("scope"), "catalog")
+               r.Equal(values.Get("scope"), scope)
 
                w.WriteHeader(http.StatusOK)
 
@@ -107,7 +108,8 @@ func (r *RestCatalogSuite) TestToken200() {
 
        cat, err := catalog.NewRestCatalog("rest", r.srv.URL,
                catalog.WithWarehouseLocation("s3://some-bucket"),
-               catalog.WithCredential(TestCreds))
+               catalog.WithCredential(TestCreds),
+               catalog.WithScope(scope))
        r.Require().NoError(err)
 
        r.NotNil(cat)