Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ func TestExternalAuthMetricsHandler_SetsMetrics(t *testing.T) {
handler := NewExternalAuthMetricsHandler(reg)

externalAuth := &api.HCPOpenShiftClusterExternalAuth{
CosmosMetadata: arm.CosmosMetadata{ResourceID: api.Must(azcorearm.ParseResourceID("/subscriptions/sub-1/resourceGroups/rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/cluster-1/externalAuths/ea-1"))},
ProxyResource: arm.ProxyResource{
Resource: arm.Resource{
ID: api.Must(azcorearm.ParseResourceID("/subscriptions/sub-1/resourceGroups/rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/cluster-1/externalAuths/ea-1")),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ func (f *externalAuthTestFixture) newCluster() *api.HCPOpenShiftCluster {

func (f *externalAuthTestFixture) newExternalAuth() *api.HCPOpenShiftClusterExternalAuth {
return &api.HCPOpenShiftClusterExternalAuth{
CosmosMetadata: arm.CosmosMetadata{ResourceID: f.externalAuthResourceID},
ProxyResource: arm.ProxyResource{
Resource: arm.Resource{
ID: f.externalAuthResourceID,
Expand Down
3 changes: 2 additions & 1 deletion backend/pkg/informers/informers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,8 @@ func controllerInformerTestCase() informerTestCase {
"/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/"+clusterName+
"/externalAuths/"+externalAuthName)
ea := &api.HCPOpenShiftClusterExternalAuth{
ProxyResource: arm.NewProxyResource(eaResourceID),
CosmosMetadata: arm.CosmosMetadata{ResourceID: eaResourceID},
ProxyResource: arm.NewProxyResource(eaResourceID),
}
_, err = mockResourcesDBClient.HCPClusters(subscriptionID, resourceGroupName).ExternalAuth(clusterName).Create(ctx, ea, nil)
require.NoError(t, err)
Expand Down
3 changes: 2 additions & 1 deletion backend/pkg/listertesting/slice_listers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,8 @@ func newTestExternalAuth(subscriptionID, resourceGroupName, clusterName, externa
"/externalAuths/" + externalAuthName,
))
return &api.HCPOpenShiftClusterExternalAuth{
ProxyResource: arm.NewProxyResource(resourceID),
CosmosMetadata: arm.CosmosMetadata{ResourceID: resourceID},
ProxyResource: arm.NewProxyResource(resourceID),
}
}

Expand Down
8 changes: 6 additions & 2 deletions frontend/pkg/frontend/external_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ func decodeDesiredExternalAuthCreate(ctx context.Context) (*api.HCPOpenShiftClus

// ProxyResource info doesn't to come from the external resource information
conversion.CopyReadOnlyProxyResourceValues(&newInternalExternalAuth.ProxyResource, ptr.To(arm.NewProxyResource(resourceID)))
newInternalExternalAuth.SetResourceID(resourceID)

// set fields that were not included during the conversion, because the user does not provide them or because the
// data is determined live on read.
Expand Down Expand Up @@ -689,8 +690,11 @@ func (f *Frontend) getInternalExternalAuthFromStorage(ctx context.Context, resou
// normalize or return a toupper or tolower form of the resource
// group or resource name. The resource group name and resource
// name must come from the URL and not the request body.
if !strings.EqualFold(internalExternalAuth.ID.String(), resourceID.String()) {
return nil, fmt.Errorf("unexpected resourceID: %s", internalExternalAuth.ID.String())
if internalExternalAuth.ResourceID == nil {
return nil, fmt.Errorf("stored externalauth document is missing cosmosMetadata.resourceID")
}
if !strings.EqualFold(internalExternalAuth.ResourceID.String(), resourceID.String()) {
return nil, fmt.Errorf("unexpected resourceID: %s", internalExternalAuth.ResourceID.String())
}
internalExternalAuth.ID = resourceID

Expand Down
19 changes: 2 additions & 17 deletions internal/api/types_externalauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package api
import (
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"

"github.com/Azure/ARO-HCP/internal/api/arm"
Expand All @@ -27,16 +26,11 @@ import (
// OpenShift clusters.
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
type HCPOpenShiftClusterExternalAuth struct {
CosmosMetadata `json:"cosmosMetadata"`

arm.ProxyResource
Properties HCPOpenShiftClusterExternalAuthProperties `json:"properties"`
ServiceProviderProperties HCPOpenShiftClusterExternalAuthServiceProviderProperties `json:"serviceProviderProperties,omitempty"`
// CosmosETag is an in-memory copy of the _etag field read from the Cosmos DB document (BaseDocument) and
// populated on DB read via the CosmosToInternalExternalAuth() conversion function.
// We carry it across the API boundary between ExternalAuth (the direct cosmos db type) and HCPOpenShiftClusterExternalAuth (this)
// so we can populate the CosmosETag in GetCosmosData() so that we can do conditional replaces in cosmos.
// This can be removed once we have inlined and serialized CosmosMetadata in
// HCPOpenShiftClusterExternalAuth.
CosmosETag azcore.ETag `json:"-"`
}

// EnsureDefaults fills in default values for fields that may be absent in
Expand All @@ -55,14 +49,6 @@ func (ea *HCPOpenShiftClusterExternalAuth) EnsureDefaults() {

var _ arm.CosmosPersistable = &HCPOpenShiftClusterExternalAuth{}

func (o *HCPOpenShiftClusterExternalAuth) GetCosmosData() *arm.CosmosMetadata {
return &arm.CosmosMetadata{
CosmosETag: o.CosmosETag,
ResourceID: o.ID,
ExistingCosmosUID: o.ServiceProviderProperties.ExistingCosmosUID,
}
}

// HCPOpenShiftClusterNodePoolProperties represents the property bag of a
// HCPOpenShiftClusterNodePool resource.
type HCPOpenShiftClusterExternalAuthProperties struct {
Expand All @@ -74,7 +60,6 @@ type HCPOpenShiftClusterExternalAuthProperties struct {
}

type HCPOpenShiftClusterExternalAuthServiceProviderProperties struct {
ExistingCosmosUID string `json:"-"`
ClusterServiceID *InternalID `json:"clusterServiceID,omitempty"`
ActiveOperationID string `json:"activeOperationId,omitempty"`
}
Expand Down
9 changes: 8 additions & 1 deletion internal/api/v20240610preview/conversion_fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ func TestRoundTripInternalExternalInternal(t *testing.T) {
func(j *azcorearm.ResourceID, c randfill.Continue) {
*j = *api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myRg"))
},
func(j *api.CosmosMetadata, c randfill.Continue) {
c.FillNoCustom(j)
// ConvertToInternal lowercases the ID, so use the lowercased canonical form
// here so the round-trip comparison succeeds.
j.ResourceID = api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/myrg"))
j.ExistingCosmosUID = ""
j.CosmosETag = ""
},
func(j *api.HCPOpenShiftClusterCustomerProperties, c randfill.Continue) {
c.FillNoCustom(j)
// ImageDigestMirrors is a v20251223preview field that does not exist in v20240610preview.
Expand Down Expand Up @@ -88,7 +96,6 @@ func TestRoundTripInternalExternalInternal(t *testing.T) {
j.ActiveOperationID = ""
// ClusterServiceID does not roundtrip through the external type because it is purely an internal detail
j.ClusterServiceID = nil
j.ExistingCosmosUID = ""
},
func(j *api.CustomerPlatformProfile, c randfill.Continue) {
c.FillNoCustom(j)
Expand Down
5 changes: 3 additions & 2 deletions internal/api/v20240610preview/external_auth_methods.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func (h *ExternalAuth) ConvertToInternal(existing *api.HCPOpenShiftClusterExtern

if h.ID != nil {
out.ID = api.Must(azcorearm.ParseResourceID(strings.ToLower(*h.ID)))
out.ResourceID = api.Must(azcorearm.ParseResourceID(strings.ToLower(*h.ID)))
}
if h.Name != nil {
out.Name = *h.Name
Expand Down Expand Up @@ -327,8 +328,8 @@ func (v version) NewHCPOpenShiftClusterExternalAuth(from *api.HCPOpenShiftCluste
}

idString := ""
if from.ID != nil {
idString = from.ID.String()
if from.ResourceID != nil {
idString = from.ResourceID.String()
}

Comment on lines 330 to 334
out := &ExternalAuth{
Expand Down
9 changes: 8 additions & 1 deletion internal/api/v20251223preview/conversion_fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ func TestRoundTripInternalExternalInternal(t *testing.T) {
func(j *azcorearm.ResourceID, c randfill.Continue) {
*j = *api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myRg"))
},
func(j *api.CosmosMetadata, c randfill.Continue) {
c.FillNoCustom(j)
// ConvertToInternal lowercases the ID, so use the lowercased canonical form
// here so the round-trip comparison succeeds.
j.ResourceID = api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/myrg"))
j.ExistingCosmosUID = ""
j.CosmosETag = ""
},
func(j *api.ImageDigestMirror, c randfill.Continue) {
c.FillNoCustom(j)
// MirrorSourcePolicy does not roundtrip through the external type because it is purely an internal detail
Expand Down Expand Up @@ -79,7 +87,6 @@ func TestRoundTripInternalExternalInternal(t *testing.T) {
j.ActiveOperationID = ""
// ClusterServiceID does not roundtrip through the external type because it is purely an internal detail
j.ClusterServiceID = nil
j.ExistingCosmosUID = ""
},
func(j *api.CustomerManagedEncryptionProfile, c randfill.Continue) {
c.FillNoCustom(j)
Expand Down
5 changes: 3 additions & 2 deletions internal/api/v20251223preview/external_auth_methods.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func (h *ExternalAuth) ConvertToInternal(existing *api.HCPOpenShiftClusterExtern

if h.ID != nil {
out.ID = api.Must(azcorearm.ParseResourceID(strings.ToLower(*h.ID)))
out.ResourceID = api.Must(azcorearm.ParseResourceID(strings.ToLower(*h.ID)))
}
if h.Name != nil {
out.Name = *h.Name
Expand Down Expand Up @@ -315,8 +316,8 @@ func (v version) NewHCPOpenShiftClusterExternalAuth(from *api.HCPOpenShiftCluste
}

idString := ""
if from.ID != nil {
idString = from.ID.String()
if from.ResourceID != nil {
idString = from.ResourceID.String()
}
Comment on lines 318 to 321

out := &ExternalAuth{
Expand Down
1 change: 1 addition & 0 deletions internal/api/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion internal/conversion/readonly_externalauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,11 @@ func CopyReadOnlyProxyResourceValues(dest, src *arm.ProxyResource) {
func CopyReadOnlyExternalAuthValues(dest, src *api.HCPOpenShiftClusterExternalAuth) {
CopyReadOnlyProxyResourceValues(&dest.ProxyResource, &src.ProxyResource)

// CosmosMetadata is read-only on the API surface; carry over so the
// case-preserving ResourceID and CosmosETag survive the replace round-trip.
dest.CosmosMetadata = *src.CosmosMetadata.DeepCopy()

Comment on lines +32 to +35
dest.Properties.ProvisioningState = src.Properties.ProvisioningState
dest.Properties.Condition = *src.Properties.Condition.DeepCopy()
dest.ServiceProviderProperties = *src.ServiceProviderProperties.DeepCopy()
dest.CosmosETag = src.CosmosETag
}
6 changes: 0 additions & 6 deletions internal/database/convert_any.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ func CosmosToInternal[InternalAPIType, CosmosAPIType any](obj *CosmosAPIType) (*
var internalObj any
var err error
switch cosmosObj := any(obj).(type) {
case *ExternalAuth:
internalObj, err = CosmosToInternalExternalAuth(cosmosObj)

case *HCPCluster:
internalObj, err = CosmosToInternalCluster(cosmosObj)

Expand Down Expand Up @@ -84,9 +81,6 @@ func InternalToCosmos[InternalAPIType, CosmosAPIType any](obj *InternalAPIType)
var cosmosObj any
var err error
switch internalObj := any(obj).(type) {
case *api.HCPOpenShiftClusterExternalAuth:
cosmosObj, err = InternalToCosmosExternalAuth(internalObj)

case *api.HCPOpenShiftCluster:
cosmosObj, err = InternalToCosmosCluster(internalObj)

Expand Down
2 changes: 1 addition & 1 deletion internal/database/convert_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func roundTripInternalToCosmosToInternal[InternalAPIType, CosmosAPIType any](t *
case *api.HCPOpenShiftClusterNodePool:
cast.ServiceProviderProperties.ExistingCosmosUID = ""
case *api.HCPOpenShiftClusterExternalAuth:
cast.ServiceProviderProperties.ExistingCosmosUID = ""
cast.ExistingCosmosUID = ""
}
//finalJSON, _ := json.MarshalIndent(final, "", " ")

Expand Down
27 changes: 14 additions & 13 deletions internal/database/convert_defaults_consistency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -577,37 +577,38 @@ func TestEnsureDefaultsConsistencyExternalAuth(t *testing.T) {
})
}

// TestPreExistingDataExternalAuth verifies that CosmosToInternalExternalAuth
// TestPreExistingDataExternalAuth verifies that CosmosGenericToInternal
// applies canonical defaults when reading a Cosmos document that predates the
// introduction of the PrefixPolicy field.
func TestPreExistingDataExternalAuth(t *testing.T) {
resourceID := api.Must(azcorearm.ParseResourceID(
"/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/cluster/externalAuths/default",
))

preExistingDoc := &ExternalAuth{
internalID := api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/test-cluster/external_auth_config/external_auths/default"))
preExistingDoc := &GenericDocument[api.HCPOpenShiftClusterExternalAuth]{
TypedDocument: TypedDocument{
BaseDocument: BaseDocument{ID: "test-doc-id"},
ResourceID: resourceID,
},
ExternalAuthProperties: ExternalAuthProperties{
IntermediateResourceDoc: &ResourceDocument{
ResourceID: resourceID,
InternalID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/test-cluster/external_auth_config/external_auths/default")),
Content: api.HCPOpenShiftClusterExternalAuth{
// PrefixPolicy is intentionally zero-valued to simulate
// a pre-existing document that predates the field.
CosmosMetadata: arm.CosmosMetadata{
ResourceID: resourceID,
},
Properties: api.HCPOpenShiftClusterExternalAuthProperties{
ProvisioningState: arm.ProvisioningStateSucceeded,
},
InternalState: ExternalAuthInternalState{
InternalAPI: api.HCPOpenShiftClusterExternalAuth{
// PrefixPolicy is intentionally zero-valued to simulate
// a pre-existing document that predates the field.
},
ServiceProviderProperties: api.HCPOpenShiftClusterExternalAuthServiceProviderProperties{
ClusterServiceID: &internalID,
},
},
}

internalExternalAuth, err := CosmosToInternalExternalAuth(preExistingDoc)
internalExternalAuth, err := CosmosGenericToInternal(preExistingDoc)
if err != nil {
t.Fatalf("CosmosToInternalExternalAuth failed: %v", err)
t.Fatalf("CosmosGenericToInternal failed: %v", err)
}

if internalExternalAuth.Properties.Claim.Mappings.Username.PrefixPolicy != api.UsernameClaimPrefixPolicyNone {
Expand Down
Loading