Skip to content

Commit

Permalink
Switch to use new Caps struct but still handle old JSON from Director
Browse files Browse the repository at this point in the history
This change fixes the stuff I broke in commit ba186d2 by handling previously-
unconsidered backwards compatibility issues. Now, unmarshalling the NSAdV2
struct will correctly populate capabilities even if the source JSON uses
the old style of capabilities.

This also fixes the capabilities displayed for each origin at the Director.
  • Loading branch information
jhiemstrawisc committed Oct 14, 2024
1 parent 11e6978 commit 5d8af33
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 6 deletions.
67 changes: 61 additions & 6 deletions server_structs/director.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,21 @@ type (
CredentialIssuer url.URL `json:"issuer"`
}

// Note that the json are kept in uppercase for backward compatibility
// A struct for unmarshalling the old JSON while we stage the breaking change across releases
OldCapabilities struct {
PublicRead bool
Read bool
Write bool
Listing bool
FallBackRead bool
}

Capabilities struct {
PublicReads bool `json:"PublicRead"`
Reads bool `json:"Read"`
Writes bool `json:"Write"`
Listings bool `json:"Listing"`
DirectReads bool `json:"FallBackRead"`
PublicReads bool
Reads bool
Writes bool
Listings bool
DirectReads bool
}

NamespaceAdV2 struct {
Expand Down Expand Up @@ -179,6 +187,53 @@ type (
}
)

// A helper function to handle JSON->NSAdV2 unmarshalling across multiple deprecated JSON keys.
//
// When the Director sends a list of NamespaceAdV2 structs as JSON, it may contain
// the old capabilities struct. This function checks if the raw JSON contains the
// "FallbackRead" field, indicating the old capabilities struct, and unmarshals
// the JSON into the new struct accordingly. This is to ensure backwards compatibility
// We can probably think about removing this function when we don't see origins/directors
// running Pelican <= v7.11.0.
func (n *NamespaceAdV2) UnmarshalJSON(data []byte) error {
// Use alias struct of NamespaceAdV2 to prevent infinite recursion of UnmarshalJSON
type Alias NamespaceAdV2
aux := &struct {
Caps json.RawMessage `json:"Caps"`
*Alias
}{
Alias: (*Alias)(n),
}

if err := json.Unmarshal(data, &aux); err != nil {
return err
}

// Check if the raw Caps JSON contains "FallbackRead", indicating
// the old caps struct. Since the struct-to-JSON code has never included
// 'omitempty', we can safely assume that the field is ALWAYS present in
// old versions of the JSON.
if strings.Contains(string(aux.Caps), "FallBackRead") {
var oldCaps OldCapabilities
if err := json.Unmarshal(aux.Caps, &oldCaps); err != nil {
return err
}

// Map old capabilities to new capabilities
n.Caps.PublicReads = oldCaps.PublicRead
n.Caps.Reads = oldCaps.Read
n.Caps.Writes = oldCaps.Write
n.Caps.Listings = oldCaps.Listing
n.Caps.DirectReads = oldCaps.FallBackRead
} else {
if err := json.Unmarshal(aux.Caps, &n.Caps); err != nil {
return err
}
}

return nil
}

func (x XPelNs) GetName() string {
return "X-Pelican-Namespace"
}
Expand Down
47 changes: 47 additions & 0 deletions server_structs/director_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package server_structs

import (
"encoding/json"
"fmt"
"net/http"
"net/url"
Expand Down Expand Up @@ -281,3 +282,49 @@ func TestXPelTokGenParsing(t *testing.T) {
assert.Len(t, xPelTokGen.BasePaths, 0)
})
}

func TestNSAdV2UnmarshalJSON(t *testing.T) {
oldJSON := `{"PublicRead":true,"Caps":{"PublicRead":true,"Read":true,"Write":false,"Listing":false,"FallBackRead":true},"path":"/ncar","token-generation":[{"strategy":"","vault-server":"","max-scope-depth":0,"issuer":{"Scheme":"","Opaque":"","User":null,"Host":"","Path":"","RawPath":"","OmitHost":false,"ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""}}],"token-issuer":[],"from-topology":true}`
newJSON := `{"Caps":{"PublicReads":false,"Reads":true,"Writes":false,"Listings":false,"DirectReads":true},"path":"/ncar","token-generation":[{"strategy":"","vault-server":"","max-scope-depth":0,"issuer":{"Scheme":"","Opaque":"","User":null,"Host":"","Path":"","RawPath":"","OmitHost":false,"ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""}}],"token-issuer":[],"from-topology":true}`

tests := []struct {
name string
jsonData string
expected Capabilities
}{
{
name: "Old JSON format",
jsonData: oldJSON,
expected: Capabilities{
PublicReads: true,
Reads: true,
Writes: false,
Listings: false,
DirectReads: true,
},
},
{
name: "New JSON format",
jsonData: newJSON,
expected: Capabilities{
PublicReads: false,
Reads: true,
Writes: false,
Listings: false,
DirectReads: true,
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var ns NamespaceAdV2
if err := json.Unmarshal([]byte(tt.jsonData), &ns); err != nil {
t.Fatalf("UnmarshalJSON() error = %v", err)
}
if ns.Caps != tt.expected {
t.Errorf("UnmarshalJSON() = %v, want %v", ns.Caps, tt.expected)
}
})
}
}

0 comments on commit 5d8af33

Please sign in to comment.