Skip to content

Commit

Permalink
Address review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jhiemstrawisc committed Feb 21, 2025
1 parent 978fc4a commit 02fe7cb
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 15 deletions.
4 changes: 2 additions & 2 deletions cache/advertise.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func getTickerRate(tok string) time.Duration {

func LaunchFedTokManager(ctx context.Context, egrp *errgroup.Group, cache server_structs.XRootDServer) {
// Do our initial token fetch+set, then turn things over to the ticker
tok, err := server_utils.GetFedTok(ctx, cache)
tok, err := server_utils.CreateFedTok(ctx, cache)
if err != nil {
log.Errorf("Failed to get a federation token: %v", err)
}
Expand All @@ -298,7 +298,7 @@ func LaunchFedTokManager(ctx context.Context, egrp *errgroup.Group, cache server
case <-fedTokTicker.C:
// Time to ask the Director for a new token
log.Debugln("Refreshing federation token")
tok, err := server_utils.GetFedTok(ctx, cache)
tok, err := server_utils.CreateFedTok(ctx, cache)
if err != nil {
log.Errorf("Failed to get a federation token: %v", err)
continue
Expand Down
10 changes: 5 additions & 5 deletions director/fed_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,26 +48,26 @@ func validateFedTokRequest(ginCtx *gin.Context) (rInfo requestInfo, err error) {
reqParams := getRequestParameters(ginCtx.Request)
hNames, exists := reqParams["host"]
if !exists || len(hNames) == 0 {
err = fmt.Errorf("no hostname found in the 'host' url parameter")
err = fmt.Errorf("no hostname found in the 'host' url parameter: %s", ginCtx.Request.URL.String())
return
} else if len(hNames) > 1 {
err = fmt.Errorf("multiple hostnames found in the 'host' url parameter")
err = fmt.Errorf("multiple hostnames found in the 'host' url parameter: %s", ginCtx.Request.URL.String())
return
}
rInfo.Host = hNames[0]

sTypes, exists := reqParams["sType"]
var sType server_structs.ServerType
if !exists || len(sTypes) == 0 {
err = fmt.Errorf("host '%s' generated request with no server type found in the 'sType' url parameter", rInfo.Host)
err = fmt.Errorf("host '%s' generated request with no server type found in the 'sType' url parameter: %s", rInfo.Host, ginCtx.Request.URL.String())
return
} else if len(sTypes) > 1 {
err = fmt.Errorf("host '%s' generated request with multiple server types in the 'sType' url parameter", rInfo.Host)
err = fmt.Errorf("host '%s' generated request with multiple server types in the 'sType' url parameter: %s", rInfo.Host, ginCtx.Request.URL.String())
return
}
valid := sType.SetString(sTypes[0])
if !valid || (sType != server_structs.CacheType && sType != server_structs.OriginType) {
err = fmt.Errorf("host '%s' generated request with invalid server type '%s' as value of 'sType' url parameter", rInfo.Host, sTypes[0])
err = fmt.Errorf("host '%s' generated request with invalid server type '%s' as value of 'sType' url parameter: %s", rInfo.Host, sTypes[0], ginCtx.Request.URL.String())
return
}
rInfo.SType = sType
Expand Down
2 changes: 1 addition & 1 deletion e2e_fed_tests/director_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func TestDirectorFedTokenCacheAPI(t *testing.T) {
ctx := context.Background()
ctx, _, _ = test_utils.TestContext(ctx, t)
cache := cache.CacheServer{}
tokStr, err := server_utils.GetFedTok(ctx, &cache)
tokStr, err := server_utils.CreateFedTok(ctx, &cache)
require.NoError(t, err, "Failed to get cache's advertisement token")
require.NotEmpty(t, tokStr, "Got an empty token")

Expand Down
13 changes: 6 additions & 7 deletions server_utils/server_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ func GetAdvertisementTok(ctx context.Context, server server_structs.XRootDServer

// GetFedTok retrieves a federation token from the Director, which can be passed to other
// federation services as proof of federation membership.
func GetFedTok(ctx context.Context, server server_structs.XRootDServer) (string, error) {
func CreateFedTok(ctx context.Context, server server_structs.XRootDServer) (string, error) {
// Set up the request to the Director
fInfo, err := config.GetFederation(ctx)
if err != nil {
Expand Down Expand Up @@ -444,17 +444,16 @@ func SetFedTok(ctx context.Context, server server_structs.XRootDServer, tok stri

dir := filepath.Dir(tokLoc)
if err := os.MkdirAll(dir, 0755); err != nil {
if !os.IsExist(err) {
return errors.Wrap(err, "failed to create fed token directories")
}
return errors.Wrap(err, "failed to create fed token directories")
}

// Create a temporary file for storing the token. Later we'll do an atomic rename
tmpName := filepath.Join(dir, fmt.Sprintf(".fedtoken.%d", time.Now().UnixNano()))
tmpFile, err := os.OpenFile(tmpName, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0600)
filenamePattern := fmt.Sprintf(".fedtoken.%d.*", time.Now().UnixNano())
tmpFile, err := os.CreateTemp(dir, filenamePattern)
if err != nil {
return errors.Wrap(err, "failed to create temporary token file")
}
tmpName := tmpFile.Name()

defer func() {
tmpFile.Close()
Expand All @@ -472,7 +471,7 @@ func SetFedTok(ctx context.Context, server server_structs.XRootDServer, tok stri
}

if err := os.Chown(tmpName, uid, gid); err != nil {
return errors.Wrap(err, "failed to change token file ownership")
return errors.Wrapf(err, "failed to change token file ownership of %s to %d:%d", tmpName, uid, gid)
}

if _, err := tmpFile.WriteString(tok); err != nil {
Expand Down

0 comments on commit 02fe7cb

Please sign in to comment.