[uss_qualifier] DSS0210,1b confirm subscription manager is synced #560

Shastick · 2024-03-14T22:13:01Z

Indirectly check that the manager of a subscription is synchronized across a DSS deployment by attempting to delete the a subscription from a DSS to which the subscription was synchronized to, using separate credentials.

Note that this only checks the case where the wrong credentials are used: confirming that deletion through any DSS works will be added with #550

Resolves #548

BenjaminPelletier · 2024-03-19T00:52:31Z

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.md

+
+##### Separate subscription
+
+Note that the subscription (or 'sub' claim, not to be confused with an SCD DSS subscription) of the token that will be obtained for this resource


sub is short for "subject" rather than "subscription"

BenjaminPelletier · 2024-03-19T00:53:44Z

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.md

+Checks that the manager of a subscription is properly synchronized across all DSS instances.
+
+This is done by means of using a separate set of credentials to create a subscription on the primary DSS,
+and then verifying that the main credentials are not able to mutate this subscription via one of the secondary DSS instances


I assume this check will be repeated for each instance?

Suggested change

and then verifying that the main credentials are not able to mutate this subscription via one of the secondary DSS instances

and then verifying that the main credentials are not able to mutate this subscription via any of the secondary DSS instances

BenjaminPelletier · 2024-03-19T00:55:08Z

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.py

@@ -63,6 +66,7 @@ def __init__(
        other_instances: DSSInstancesResource,
        id_generator: IDGeneratorResource,
        planning_area: PlanningAreaResource,
+        second_utm_auth: Optional[AuthAdapterResource],


I think this needs a default value (presumably None) to actually be considered optional by the framework

Ah, I get why certain things confused me now. Thanks.

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.py

BenjaminPelletier · 2024-03-19T01:00:43Z

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.md

+
+Verify that a subscription can be created on the primary DSS.
+
+#### 🛑 Subscription deletion with different non-managing credentials on secondary DSS fails check


If this check fails, doesn't that mean the subscription has been deleted so we should be able to simply proceed with the rest of the test (after skipping the next step since the subscription is already deleted)? If so, this should probably be a Medium-severity issue.

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.md

BenjaminPelletier · 2024-03-19T01:14:37Z

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.py

+                if deleted_sub.status_code != 403:
+                    check.record_failed(
+                        "Subscription deletion with main credentials did not fail",
+                        details=f"Subscription deletion with main credentials failed with status code {deleted_sub.status_code}",


Suggested change

details=f"Subscription deletion with main credentials failed with status code {deleted_sub.status_code}",

details=f"Subscription deletion with main credentials did not fail with the expected status code of 403; instead returned status code {deleted_sub.status_code}",

BenjaminPelletier · 2024-03-19T01:23:26Z

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.py

@@ -87,6 +91,7 @@ def __init__(
        ]

        self._sub_id = id_generator.id_factory.make_id(self.SUB_TYPE)
+        self._acl_sub_id = self._sub_id[:-1] + "1"


Let's not make ad-hoc IDs; the reason to identify the purpose of the ID when calling make_id is to allow people to reverse-lookup the test that caused a particular object to be added to the DSS.

Side question for situations where we might want to generate a dynamic (or configurable) number of test IDs: could we update the reverse lookup logic to support derived IDs where we increment the last block of zeroes on the ID?

Yes, I think that makes sense. And IDs without reverse lookup are not the most important issue to deal with -- I just want to avoid adding more tech debt when practical.

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.py

BenjaminPelletier

Just a few more comments on documentation but then I think this one's ready to go

BenjaminPelletier · 2024-03-20T17:28:48Z

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.md

+
+This second set of credentials is required to validate that the DSS properly synchronizes the manager of a subscription to other DSS instances.
+
+The participant under test is responsible for providing this second set of credentials along the primary ones used in most other scenarios.


The test designer sets up the configuration which provides these resources and is therefore responsible for providing any needed resources; "participants" are the subjects of the test (e.g., USSs under test) and would not provide credentials or any other resource (though they would communicate with the test designer to ensure the test designer specifies the URLs of the USS under test correctly in the environmental resources).

Suggested change

The participant under test is responsible for providing this second set of credentials along the primary ones used in most other scenarios.

The test designer should provide this second set of credentials when full testing of manager synchronization behavior is desired.

BenjaminPelletier · 2024-03-20T17:45:20Z

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.md

+For the purpose of this scenario, these credentials must be allowed to create, modify and delete subscriptions on the DSS,
+as well as querying them.


This statement is redundant with the "Required scope" section since a scope determines what actions a client is allowed to perform. So, I think these lines can be removed (otherwise, we would need to explain how someone would know if the credentials are allowed to create, modify and delete subscriptions in the DSS as well as querying them)

BenjaminPelletier · 2024-03-20T17:45:48Z

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.md

+
+* `utm.strategic_coordination`
+
+##### Separate subscription


Separate subject?

Oh my, yes indeed that needs fixing too.

BenjaminPelletier · 2024-03-20T19:38:19Z

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.md

@@ -145,3 +199,8 @@ If a DSS returns a subscription that was previously successfully deleted from th
 either one of the primary DSS or the DSS that returned the subscription is in violation of **[astm.f3548.v21.DSS0210,1a](../../../../../requirements/astm/f3548/v21.md)**.

 ## [Cleanup](../clean_workspace.md)
+
+This step ensures that no subscriptions with the known test IDs exists in the DSS.


nit:

Suggested change

This step ensures that no subscriptions with the known test IDs exists in the DSS.

This step ensures that no subscriptions with the known test IDs remain in the DSS by deleting them at this point if they do exist.

Shastick · 2024-03-20T21:10:12Z

Just a few more comments on documentation but then I think this one's ready to go

Thanks!

While starting to integrate with the lastest state of main, I remembered that the subscription synchronization scenario still needed to be adapted to the conventions we converged to in #535:

I did this in #570. The present PR should probably come after it as it.

(Note: the present PR rebases without conflict, but some check names unrelated to this PR need to be changed)

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.md

* [uss_qualifier] DSS0210,1b confirm subscription manager is synced * latest PR comments * 2nd round of review 951bf95

Shastick requested a review from barroco March 15, 2024 08:19

BenjaminPelletier reviewed Mar 19, 2024

View reviewed changes

Shastick force-pushed the dss0210-sub-manager-synced branch 2 times, most recently from 0e0cac3 to 7dbf130 Compare March 19, 2024 11:28

Shastick requested a review from BenjaminPelletier March 19, 2024 11:29

Shastick force-pushed the dss0210-sub-manager-synced branch from 9dcb897 to 849b534 Compare March 19, 2024 16:48

BenjaminPelletier reviewed Mar 20, 2024

View reviewed changes

Shastick force-pushed the dss0210-sub-manager-synced branch from 849b534 to 2f4de7e Compare March 20, 2024 21:07

Shastick force-pushed the dss0210-sub-manager-synced branch 2 times, most recently from 9df3732 to db909ca Compare March 20, 2024 21:19

Shastick marked this pull request as draft March 20, 2024 23:24

Shastick force-pushed the dss0210-sub-manager-synced branch from db909ca to 8a8f82b Compare March 21, 2024 08:16

Shastick marked this pull request as ready for review March 21, 2024 10:36

Shastick requested a review from BenjaminPelletier March 21, 2024 10:36

Shastick force-pushed the dss0210-sub-manager-synced branch from 8a8f82b to 0549f93 Compare March 21, 2024 10:39

Shastick commented Mar 21, 2024

View reviewed changes

monitoring/uss_qualifier/scenarios/astm/utm/dss/synchronization/subscription_synchronization.md Show resolved Hide resolved

Shastick force-pushed the dss0210-sub-manager-synced branch 3 times, most recently from 6433875 to 9b46e71 Compare March 21, 2024 16:24

Shastick added 3 commits March 22, 2024 16:37

[uss_qualifier] DSS0210,1b confirm subscription manager is synced

60161a7

latest PR comments

643bda2

2nd round of review

26c7ed6

Shastick force-pushed the dss0210-sub-manager-synced branch from 3ef83fd to 26c7ed6 Compare March 22, 2024 15:40

BenjaminPelletier approved these changes Mar 22, 2024

View reviewed changes

BenjaminPelletier merged commit 951bf95 into interuss:main Mar 22, 2024
9 checks passed

Shastick deleted the dss0210-sub-manager-synced branch March 22, 2024 16:37

github-actions bot added a commit that referenced this pull request Mar 22, 2024

[uss_qualifier] DSS0210,1b confirm subscription manager is synced (#560)

111b430

* [uss_qualifier] DSS0210,1b confirm subscription manager is synced * latest PR comments * 2nd round of review 951bf95

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[uss_qualifier] DSS0210,1b confirm subscription manager is synced #560

[uss_qualifier] DSS0210,1b confirm subscription manager is synced #560

Shastick commented Mar 14, 2024

BenjaminPelletier Mar 19, 2024

BenjaminPelletier Mar 19, 2024

Shastick Mar 19, 2024

BenjaminPelletier Mar 19, 2024

Shastick Mar 19, 2024

BenjaminPelletier Mar 19, 2024

Shastick Mar 19, 2024

BenjaminPelletier Mar 19, 2024

BenjaminPelletier Mar 19, 2024

Shastick Mar 19, 2024

BenjaminPelletier Mar 20, 2024

BenjaminPelletier left a comment

BenjaminPelletier Mar 20, 2024

BenjaminPelletier Mar 20, 2024

BenjaminPelletier Mar 20, 2024

Shastick Mar 20, 2024

BenjaminPelletier Mar 20, 2024

Shastick commented Mar 20, 2024 •

edited

Loading


		##### Separate subscription

		Note that the subscription (or 'sub' claim, not to be confused with an SCD DSS subscription) of the token that will be obtained for this resource

	and then verifying that the main credentials are not able to mutate this subscription via one of the secondary DSS instances
	and then verifying that the main credentials are not able to mutate this subscription via any of the secondary DSS instances


		Verify that a subscription can be created on the primary DSS.

		#### 🛑 Subscription deletion with different non-managing credentials on secondary DSS fails check

	details=f"Subscription deletion with main credentials failed with status code {deleted_sub.status_code}",
	details=f"Subscription deletion with main credentials did not fail with the expected status code of 403; instead returned status code {deleted_sub.status_code}",


		This second set of credentials is required to validate that the DSS properly synchronizes the manager of a subscription to other DSS instances.

		The participant under test is responsible for providing this second set of credentials along the primary ones used in most other scenarios.

	The participant under test is responsible for providing this second set of credentials along the primary ones used in most other scenarios.
	The test designer should provide this second set of credentials when full testing of manager synchronization behavior is desired.

		For the purpose of this scenario, these credentials must be allowed to create, modify and delete subscriptions on the DSS,
		as well as querying them.

	This step ensures that no subscriptions with the known test IDs exists in the DSS.
	This step ensures that no subscriptions with the known test IDs remain in the DSS by deleting them at this point if they do exist.

[uss_qualifier] DSS0210,1b confirm subscription manager is synced #560

[uss_qualifier] DSS0210,1b confirm subscription manager is synced #560

Conversation

Shastick commented Mar 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenjaminPelletier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shastick commented Mar 20, 2024 • edited Loading

Shastick commented Mar 20, 2024 •

edited

Loading