Skip to content

Commit

Permalink
Merge pull request opendatahub-io#2899 from DaoDaoNoCode/jira-rhoaien…
Browse files Browse the repository at this point in the history
…g-3782

Remove DS pipeline endpoint hack
  • Loading branch information
openshift-merge-bot[bot] authored Jun 12, 2024
2 parents fa1489f + b529891 commit 7d5b99f
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ describe('Pipelines', () => {
dspVersion: 'v2',
objectStorage: {
externalStorage: {
host: 's3.us-east-1.amazonaws.com',
host: 's3.amazonaws.com/',
scheme: 'https',
bucket: 'sdsd',
region: 'us-east-1',
Expand Down Expand Up @@ -167,7 +167,7 @@ describe('Pipelines', () => {
pipelinesGlobal.findConfigurePipelineServerButton().click();
configurePipelineServerModal.findAwsKeyInput().type('test-aws-key');
configurePipelineServerModal.findAwsSecretKeyInput().type('test-secret-key');
configurePipelineServerModal.findEndpointInput().type('https://s3.amazonaws.com/');
configurePipelineServerModal.findEndpointInput().type('https://s3.amazonaws.com');
configurePipelineServerModal.findRegionInput().should('have.value', 'us-east-1');
configurePipelineServerModal.findBucketInput().type('test-bucket');
configurePipelineServerModal.findSubmitButton().should('be.enabled');
Expand Down Expand Up @@ -202,7 +202,7 @@ describe('Pipelines', () => {
dspVersion: 'v2',
objectStorage: {
externalStorage: {
host: 's3.us-east-1.amazonaws.com',
host: 's3.amazonaws.com',
scheme: 'https',
bucket: 'test-bucket',
region: 'us-east-1',
Expand Down Expand Up @@ -244,7 +244,7 @@ describe('Pipelines', () => {

configurePipelineServerModal.findAwsKeyInput().type('test-aws-key');
configurePipelineServerModal.findAwsSecretKeyInput().type('test-secret-key');
configurePipelineServerModal.findEndpointInput().type('https://s3.amazonaws.com/');
configurePipelineServerModal.findEndpointInput().type('https://s3.amazonaws.com');
configurePipelineServerModal.findRegionInput().should('have.value', 'us-east-1');
configurePipelineServerModal.findBucketInput().type('test-bucket');

Expand Down Expand Up @@ -302,7 +302,7 @@ describe('Pipelines', () => {
dspVersion: 'v2',
objectStorage: {
externalStorage: {
host: 's3.us-east-1.amazonaws.com',
host: 's3.amazonaws.com',
scheme: 'https',
bucket: 'test-bucket',
region: 'us-east-1',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ describe('configure pipeline server utils', () => {
];
const spec = createDSPipelineResourceSpec(config, secretsResponse);
expect(spec.objectStorage.externalStorage?.scheme).toBe('http');
expect(spec.objectStorage.externalStorage?.host).toBe('s3.us-east-1.amazonaws.com');
expect(spec.objectStorage.externalStorage?.host).toBe('s3.amazonaws.com');
});

it('should parse S3 endpoint without scheme', () => {
Expand All @@ -77,30 +77,7 @@ describe('configure pipeline server utils', () => {
config.objectStorage.newValue = [{ key: AwsKeys.S3_ENDPOINT, value: 's3.amazonaws.com' }];
const spec = createDSPipelineResourceSpec(config, secretsResponse);
expect(spec.objectStorage.externalStorage?.scheme).toBe('https');
expect(spec.objectStorage.externalStorage?.host).toBe('s3.us-east-1.amazonaws.com');
});

it('should convert S3 endpoint with region', () => {
const config = createPipelineServerConfig();
const secretsResponse = createSecretsResponse();
config.objectStorage.newValue = [
{ key: AwsKeys.S3_ENDPOINT, value: 'http://s3.amazonaws.com' },
{ key: AwsKeys.DEFAULT_REGION, value: 'us-east-2' },
];
const spec = createDSPipelineResourceSpec(config, secretsResponse);
expect(spec.objectStorage.externalStorage?.scheme).toBe('http');
expect(spec.objectStorage.externalStorage?.host).toBe('s3.us-east-2.amazonaws.com');
});

it('should not convert endpoint when it is not S3', () => {
const config = createPipelineServerConfig();
const secretsResponse = createSecretsResponse();
config.objectStorage.newValue = [
{ key: AwsKeys.S3_ENDPOINT, value: 'http://s3.not-amazonaws.com' },
];
const spec = createDSPipelineResourceSpec(config, secretsResponse);
expect(spec.objectStorage.externalStorage?.scheme).toBe('http');
expect(spec.objectStorage.externalStorage?.host).toBe('s3.not-amazonaws.com');
expect(spec.objectStorage.externalStorage?.host).toBe('s3.amazonaws.com');
});

it('should include bucket', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export const createDSPipelineResourceSpec = (
dspVersion: 'v2',
objectStorage: {
externalStorage: {
host: convertAWSHostForRegion(externalStorageHost, externalStorageRegion),
host: externalStorageHost,
scheme: externalStorageScheme || 'https',
bucket: awsRecord.AWS_S3_BUCKET || '',
region: externalStorageRegion,
Expand Down Expand Up @@ -157,8 +157,3 @@ export const getLabelName = (index: string): string => {
const field = PIPELINE_AWS_FIELDS.find((currentField) => currentField.key === index);
return field ? field.label : '';
};

const convertAWSHostForRegion = (endpoint: string, region: string): string => {
const host = endpoint.replace(/\/$/, '') || '';
return host === 's3.amazonaws.com' && region !== '' ? `s3.${region}.amazonaws.com` : host;
};

0 comments on commit 7d5b99f

Please sign in to comment.