Multiple Fixes Following Failed S3 Tests

[nadavMiz]

Explain the Changes

small fixes for s3 tests.
since all the changes are small. the are are in the same pull request but different commits. there are three commits. one for each change:

1. add condition to check limit for number of deleted objects is larger than 1000 as specified in aws specification - 7b7abb1
2. changed error returned for invalid encryption algorithm to InvalidArgument - cf2869a
3. fix test job regex to match against full test names and not substring - 82cd977
4. add lifecycle rule to pending list status as faulty. s3-tests issue (510) - 4653dac

Issues:

no related issues

Testing Instructions:

run ceph s3 tests using the following instructions:
https://github.com/noobaa/noobaa-core/blob/master/docs/dev_guide/ceph_s3_tests/ceph_s3_tests_guide.md

run the following s3 tests:

1. s3tests_boto3.functional.test_s3:test_multi_object_delete_key_limit
2. s3tests_boto3.functional.test_s3:test_lifecycle_set_date
3. s3tests_boto3.functional.test_s3:test_put_obj_enc_conflict_bad_enc_kms

• Doc added/updated
• Tests added

[shirady]

Hi @nadavMiz,
Did you notice that test s3tests_boto3.functional.test_s3.test_lifecycle_transition_set_invalid_date is failing in the CI?

[nimrod-becker]

Regarding Q2, since the changed were small I suggested a single PR with 3 commits

[shirady]

Regarding Q2, since the changes were small I suggested a single PR with 3 commits

@nimrod-becker I understood t after the code review.
@nadavMiz, Would you please edit the description of the PR with this explanation?

[nadavMiz]

regarding test_lifecycle_transition_set_invalid_date, it looks like it never really passed. it checks if set date returns and exception using prefix and not filter. since we didn't allow to have prefix in the request we sent an error, when fixing this for set date, test_lifecycle_transition_set_invalid_date passed that point and the test failed. I added this test to the pending list

[shirady]

@baum, would you please also review the changes in this file?

[baum]

Great job on the update! 🖖
It would be beneficial to include a positive test case for the Rule with prefix and a negative test case for the Rule with both prefix and filter. Currently, there is a recipe that involves

• adding lifecycle configuration instance see here (

  noobaa-core/src/test/lifecycle/common.js

  Lines 359 to 376 in 57b0dde

  	function id_lifecycle_configuration(Bucket, Key) {
  	const ID = 'rule_id';
  	return {
  	Bucket,
  	LifecycleConfiguration: {
  	Rules: [{
  	ID,
  	Expiration: {
  	Days: 17,
  	},
  	Filter: {
  	Prefix: Key,
  	},
  	Status: 'Enabled',
  	}, ],
  	},
  	};
  	}

  )
• defining a common test function (

  noobaa-core/src/test/lifecycle/common.js

  Lines 497 to 506 in 57b0dde

  	exports.test_rule_id = async function(Bucket, Key, s3) {
  	const putLifecycleParams = id_lifecycle_configuration(Bucket, Key);
  	const getLifecycleResult = await put_get_lifecycle_configuration(Bucket, putLifecycleParams, s3);
  	const actualId = getLifecycleResult.Rules[0].ID;
  	const expectedId = putLifecycleParams.LifecycleConfiguration.Rules[0].ID;
  	console.log('get rule id:', actualId, ' expected:', expectedId);
  	assert.deepEqual(actualId, expectedId, 'rule id');
  	};

  )
• adding it to the list of unit tests (

  noobaa-core/src/test/unit_tests/test_lifecycle.js

  Lines 75 to 77 in 57b0dde

  	mocha.it('test rule id', async () => {
  	await commonTests.test_rule_id(Bucket, Key, s3);
  	});

  )
• and system tests (

  noobaa-core/src/test/system_tests/test_lifecycle.js

  Line 54 in 57b0dde

  await commonTests.test_rule_id(Bucket, Key, s3);

  ).

It is recommended to run the system lifecycle test manually to compare the lifecycle configuration behavior of AWS S3 and NooBaa endpoints.

[nadavMiz]

@baum I made the tests as requested: lifecycle commit. can you please review them

[shirady]

@nadavMiz in the CI I see it runs 388 passed tests instead of 389 tests (387 we have today + 3 you inserted - 1 you removed is 389 but it is written 388):
Apr-19 14:59:14.653 [test_ceph_s3/25] [INFO] CONSOLE:: CEPH TEST SUMMARY: Suite contains 812, ran 388 tests, Passed: 388, Skipped: 0, Failed: 0
I didn't find the test test_lifecycle_set_date in the logs.
Would you please check this?

[shirady]

LGTM
Great fixes!
I didn't review the added unit tests @baum asked for.

[baum]

Thank you, lgtm 🖖

[liranmauda]

please squash

[shirady]

@liranmauda, those are different fixes and could be in different PRs.
@nimrod-becker asked for one PR with a couple of commits as I understand.

[nimrod-becker]

Ack on Shira's comment. 3 different fixes

[shirady]

Ack on Shira's comment. 3 different fixes

I'll add that the 4th commit is a fix for an issue we found during this PR in the script running all tests (not a specific test).

disimissing due to #7270 (comment)

should net be blocked....