Skip to content
This repository has been archived by the owner on Jul 4, 2024. It is now read-only.

E2E dest fetcher fixes #3852

Merged
merged 12 commits into from
Apr 29, 2024
Merged
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
10 changes: 10 additions & 0 deletions chart/compass/templates/tests/director/director-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,8 @@ spec:
key: {{ .Values.global.tests.subscription.destinationOauthSecret.dependencyKey }}
- name: APP_DESTINATION_CONSUMER_SUBDOMAIN
value: {{ .Values.global.tests.destination.consumerSubdomain | quote }}
- name: APP_DESTINATION_CONSUMER_SUBDOMAIN_MTLS
value: {{ .Values.global.tests.destination.consumerSubdomainMtls | quote }}
- name: APP_TEST_DESTINATION_INSTANCE_ID
value: {{ .Values.global.tests.destination.instanceID | quote }}
- name: APP_USER_AUTHENTICATOR_ACCOUNT_CLIENT_ID
Expand Down Expand Up @@ -661,6 +663,8 @@ spec:
key: {{ .Values.global.tests.subscription.destinationOauthSecret.dependencyKey }}
- name: APP_DESTINATION_CONSUMER_SUBDOMAIN
value: {{ .Values.global.tests.destination.consumerSubdomain | quote }}
- name: APP_DESTINATION_CONSUMER_SUBDOMAIN_MTLS
value: {{ .Values.global.tests.destination.consumerSubdomainMtls | quote }}
- name: APP_TEST_DESTINATION_INSTANCE_ID
value: {{ .Values.global.tests.destination.instanceID | quote }}
- name: APP_USER_AUTHENTICATOR_ACCOUNT_CLIENT_ID
Expand Down Expand Up @@ -1023,6 +1027,8 @@ spec:
key: {{ .Values.global.tests.subscription.destinationOauthSecret.dependencyKey }}
- name: APP_DESTINATION_CONSUMER_SUBDOMAIN
value: {{ .Values.global.tests.destination.consumerSubdomain | quote }}
- name: APP_DESTINATION_CONSUMER_SUBDOMAIN_MTLS
value: {{ .Values.global.tests.destination.consumerSubdomainMtls | quote }}
- name: APP_TEST_DESTINATION_INSTANCE_ID
value: {{ .Values.global.tests.destination.instanceID | quote }}
- name: APP_USER_AUTHENTICATOR_ACCOUNT_CLIENT_ID
Expand Down Expand Up @@ -1385,6 +1391,8 @@ spec:
key: {{ .Values.global.tests.subscription.destinationOauthSecret.dependencyKey }}
- name: APP_DESTINATION_CONSUMER_SUBDOMAIN
value: {{ .Values.global.tests.destination.consumerSubdomain | quote }}
- name: APP_DESTINATION_CONSUMER_SUBDOMAIN_MTLS
value: {{ .Values.global.tests.destination.consumerSubdomainMtls | quote }}
- name: APP_TEST_DESTINATION_INSTANCE_ID
value: {{ .Values.global.tests.destination.instanceID | quote }}
- name: APP_USER_AUTHENTICATOR_ACCOUNT_CLIENT_ID
Expand Down Expand Up @@ -1747,6 +1755,8 @@ spec:
key: {{ .Values.global.tests.subscription.destinationOauthSecret.dependencyKey }}
- name: APP_DESTINATION_CONSUMER_SUBDOMAIN
value: {{ .Values.global.tests.destination.consumerSubdomain | quote }}
- name: APP_DESTINATION_CONSUMER_SUBDOMAIN_MTLS
value: {{ .Values.global.tests.destination.consumerSubdomainMtls | quote }}
- name: APP_TEST_DESTINATION_INSTANCE_ID
value: {{ .Values.global.tests.destination.instanceID | quote }}
- name: APP_USER_AUTHENTICATOR_ACCOUNT_CLIENT_ID
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ spec:
value: "{{ .Values.global.destinationFetcher.prefix }}{{ .Values.global.destinationFetcher.server.tenantInstanceLevelDestinationCertificatesEndpoint }}"
- name: APP_DESTINATION_CONSUMER_SUBDOMAIN
value: {{ .Values.global.tests.destination.consumerSubdomain | quote }}
- name: APP_DESTINATION_CONSUMER_SUBDOMAIN_MTLS
value: {{ .Values.global.tests.destination.consumerSubdomainMtls | quote }}
- name: TEST_EXTERNAL_CERT_CN
value: {{ .Values.global.externalCertConfiguration.commonName }}
- name: TEST_EXTERNAL_CERT_SUBJECT
Expand Down
7 changes: 4 additions & 3 deletions chart/compass/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ global:
name: compass-pairing-adapter
director:
dir: dev/incubator/
version: "PR-3853"
version: "PR-3852"
name: compass-director
hydrator:
dir: dev/incubator/
Expand Down Expand Up @@ -224,11 +224,11 @@ global:
version: "0a651695"
external_services_mock:
dir: dev/incubator/
version: "PR-3841"
version: "PR-3852"
name: compass-external-services-mock
e2e_tests:
dir: dev/incubator/
version: "PR-3853"
version: "PR-3852"
name: compass-e2e-tests
isLocalEnv: false
isForTesting: false
Expand Down Expand Up @@ -1341,6 +1341,7 @@ global:
region: "eu-1"
destination:
consumerSubdomain: "compass-external-services-mock"
consumerSubdomainMtls: "compass-external-services-mock-sap-mtls"
instanceID: "37d7d783-d9ad-47de-b6c8-b05a4cb961ca" # randomly generated UUID
claims:
subaccountIDKey: "subaccountid"
Expand Down
41 changes: 34 additions & 7 deletions components/director/internal/destinationfetchersvc/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,33 @@ type destinationFromService struct {
XFSystemName string `json:"XFSystemName"`
CommunicationScenarioID string `json:"communicationScenarioId"`
ProductName string `json:"product.name"`
CorrelationIDs string `json:"correlationIds"`
XCorrelationID string `json:"x-correlation-id"`
XSystemTenantID string `json:"x-system-id"`
XSystemTenantName string `json:"x-system-name"`
XSystemType string `json:"x-system-type"`
XSystemBaseURL string `json:"x-system-base-url"`
}

// Validate returns error if system doesn't have the required properties
func (d *destinationFromService) Validate() error {
if d.XCorrelationID == "" && d.CorrelationIDs == "" {
return errors.New("missing destination correlation id")
}

if d.XCorrelationID != "" && d.CorrelationIDs != "" {
return errors.New("old and new correlation ID format provided")
}

return nil
}

func (d *destinationFromService) handleDefaultCorrelationID() {
if d.XCorrelationID == "" && d.CorrelationIDs == "" && d.CommunicationScenarioID != "" {
d.XCorrelationID = correlationIDPrefix + d.CommunicationScenarioID
}
}

func (d *destinationFromService) setDefaults(result *model.DestinationInput) error {
// Set values from custom properties
if result.XSystemType == "" {
Expand All @@ -85,11 +105,7 @@ func (d *destinationFromService) setDefaults(result *model.DestinationInput) err
if result.XSystemType != s4HANAType {
return nil
}
if result.XCorrelationID == "" {
if d.CommunicationScenarioID != "" {
result.XCorrelationID = correlationIDPrefix + d.CommunicationScenarioID
}
}

if result.XSystemTenantName == "" {
result.XSystemTenantName = d.XFSystemName
}
Expand All @@ -114,12 +130,23 @@ func (d *destinationFromService) setDefaults(result *model.DestinationInput) err

// ToModel missing godoc
func (d *destinationFromService) ToModel() (model.DestinationInput, error) {
d.handleDefaultCorrelationID()

if err := d.Validate(); err != nil {
return model.DestinationInput{}, err
}

correlationIDs := d.CorrelationIDs
if d.XCorrelationID != "" {
correlationIDs = d.XCorrelationID
}

result := model.DestinationInput{
Name: d.Name,
Type: d.Type,
URL: d.URL,
Authentication: d.Authentication,
XCorrelationID: d.XCorrelationID,
XCorrelationID: correlationIDs,
XSystemTenantID: d.XSystemTenantID,
XSystemTenantName: d.XSystemTenantName,
XSystemType: d.XSystemType,
Expand All @@ -130,7 +157,7 @@ func (d *destinationFromService) ToModel() (model.DestinationInput, error) {
return model.DestinationInput{}, err
}

return result, result.Validate()
return result, nil
}

// DestinationResponse paged response from destination service
Expand Down
15 changes: 1 addition & 14 deletions components/director/internal/model/destination.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
package model

import (
"errors"
)

// DestinationInput missing godoc
type DestinationInput struct {
Name string `json:"Name"`
Type string `json:"Type"`
URL string `json:"URL"`
Authentication string `json:"Authentication"`
XCorrelationID string `json:"x-correlation-id"`
XCorrelationID string `json:"correlationIds"`
XSystemTenantID string `json:"x-system-id"`
XSystemTenantName string `json:"x-system-name"`
XSystemType string `json:"x-system-type"`
Expand All @@ -29,15 +25,6 @@ type Destination struct {
FormationAssignmentID *string `json:"formationAssignmentID"`
}

// Validate returns error if system doesn't have the required properties
func (d *DestinationInput) Validate() error {
if d.XCorrelationID == "" {
return errors.New("missing destination correlation id")
}

return nil
}

// HasValidIdentifiers checks if the destination has either one of the pairs: XSystemTenantID and XSystemType, or XSystemBaseURL and XSystemTenantName
func (d *DestinationInput) HasValidIdentifiers() bool {
hasSystemIDAndType := d.XSystemTenantID != "" && d.XSystemType != ""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/base64"
"encoding/json"
"fmt"
"github.com/tidwall/sjson"
"io"
"net/http"
"strconv"
Expand Down Expand Up @@ -316,17 +317,16 @@ func (h *Handler) PostDestination(writer http.ResponseWriter, req *http.Request)
authType = authResult.String()
}

tokenValue, err := validateAuthorization(ctx, req)
if err != nil {
httphelpers.RespondWithError(ctx, writer, err, err.Error(), correlationID, http.StatusUnauthorized)
return
}
subaccountID := req.Header.Get("subaccount_id")
serviceInstanceID := req.Header.Get("instance_id")

if subaccountID == "" && serviceInstanceID == "" {
err := errors.New("missing subaccount_id and instance_id headers")
httphelpers.RespondWithError(ctx, writer, err, err.Error(), correlationID, http.StatusBadRequest)

subaccountID, serviceInstanceID, err := extractSubaccountIDAndServiceInstanceIDFromDestinationToken(tokenValue)
if err != nil {
httphelpers.RespondWithError(ctx, writer, err, err.Error(), correlationID, http.StatusInternalServerError)
return
}

log.C(ctx).Infof("Subaccount ID: %q and service instance ID: %q in the destination token", subaccountID, serviceInstanceID)

var responses []PostResponse
Expand Down Expand Up @@ -385,7 +385,6 @@ func (h *Handler) PostDestination(writer http.ResponseWriter, req *http.Request)
// DeleteDestination is an "internal/technical" handler for deleting Destinations from E2E tests
func (h *Handler) DeleteDestination(writer http.ResponseWriter, req *http.Request) {
ctx := req.Context()
correlationID := correlation.CorrelationIDFromContext(ctx)

destinationName := mux.Vars(req)["name"]

Expand All @@ -394,17 +393,13 @@ func (h *Handler) DeleteDestination(writer http.ResponseWriter, req *http.Reques
return
}

tokenValue, err := validateAuthorization(ctx, req)
if err != nil {
httphelpers.RespondWithError(ctx, writer, err, err.Error(), correlationID, http.StatusUnauthorized)
subaccountID := req.Header.Get("subaccount_id")
serviceInstanceID := req.Header.Get("instance_id")
if subaccountID == "" && serviceInstanceID == "" {
http.Error(writer, "Bad request - missing subaccount_id and instance_id headers", http.StatusBadRequest)
return
}

subaccountID, serviceInstanceID, err := extractSubaccountIDAndServiceInstanceIDFromDestinationToken(tokenValue)
if err != nil {
httphelpers.RespondWithError(ctx, writer, err, err.Error(), correlationID, http.StatusInternalServerError)
return
}
log.C(ctx).Infof("Deleting Destination with subaccount ID: %q and service instance ID: %q in the destination token", subaccountID, serviceInstanceID)

identifiers := map[string]string{
Expand Down Expand Up @@ -879,6 +874,14 @@ func requestBodyToDestination(authType string, bodyBytes []byte) (DestinationReq
return nil, errors.Errorf("The provided destination authentication type: %s is invalid", authType)
}

var err error
if correlationIds := gjson.GetBytes(bodyBytes, "additionalProperties.correlationIds"); correlationIds.Exists() {
bodyBytes, err = sjson.SetBytes(bodyBytes, "correlationIds", correlationIds.String())
if err != nil {
return nil, err
}
}

destinationTypeName := destinationRequestBody.GetDestinationType()
if err := json.Unmarshal(bodyBytes, &destinationRequestBody); err != nil {
return nil, errors.Wrapf(err, "An error occurred while unmarshalling %s destination request body", destinationTypeName)
Expand Down
Loading