Skip to content

Expose Scan:: registry/token: Change istio authorization policies to allow traffic to the registry endpoints#5970

Open
pasindutennage-da wants to merge 4 commits into
mainfrom
pasindutennage-da/fixi/ssue/5946
Open

Expose Scan:: registry/token: Change istio authorization policies to allow traffic to the registry endpoints#5970
pasindutennage-da wants to merge 4 commits into
mainfrom
pasindutennage-da/fixi/ssue/5946

Conversation

@pasindutennage-da

Copy link
Copy Markdown
Contributor

Fix #5946

[ci]

Signed-off-by: pasindutennage-da <pasindu.tennage@digitalasset.com>

Signed-off-by: Pasindu Tennage <pasindu.tennage@digitalasset.com>
[ci]

Signed-off-by: pasindutennage-da <pasindu.tennage@digitalasset.com>

Signed-off-by: Pasindu Tennage <pasindu.tennage@digitalasset.com>
[ci]

Signed-off-by: pasindutennage-da <pasindu.tennage@digitalasset.com>

Signed-off-by: Pasindu Tennage <pasindu.tennage@digitalasset.com>
@canton-network-da

Copy link
Copy Markdown
Contributor

[backport] Reminder

Please consider backporting to the following branches:

  • release-line-0.6.8
  • release-line-0.6.7
  • release-line-0.6.6
  • release-line-0.6.5

▶️ Please check the boxes for branches that you wish to backport to and backport PRs will
automatically be created when you merge this PR.

And your PR is currently against base branch: main.

Note: Any PR comment containing [backport] will be considered for auto-backporting upon merge,
you can always add those manually for PRs that did not get these reminders. You can also edit
this comment manually and add more branches that this should be backported to.

[ci]

Signed-off-by: pasindutennage-da <pasindu.tennage@digitalasset.com>

Signed-off-by: Pasindu Tennage <pasindu.tennage@digitalasset.com>
)
muteTimeIntervals: [ ]
cloudArmor:
enabled: false

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when should this be changed?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably a question for @stephencompall-DA

maxRequestsBeforeHttp429: 0
maxRequestsBeforeHttp429: 0 # Keeps scan completely closed to the public
tokenRegistry:
hostname: scan.sv-2.scratcha.global.canton.network.digitalasset.com

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what should we set here / or should this be set in pulumi dynamically?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hardcoding definitely won't work, so either you get rid of the need to set it or you need to set it from pulumi

@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown

maxTokens: 10
tokensPerFill: 5
fillInterval: 60s
/registry/allocations/v1:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm a bit unfortunate that we need to list all endpoints individually here. iirc we had some safeguards somewhere that ensures that we don't miss endpoints, can we extend them to cover this. In particular I'd like to avoid that we break this for token std v2.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah I see you've done that below already

)
muteTimeIntervals: [ ]
cloudArmor:
enabled: false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably a question for @stephencompall-DA

maxRequestsBeforeHttp429: 0
maxRequestsBeforeHttp429: 0 # Keeps scan completely closed to the public
tokenRegistry:
hostname: scan.sv-2.scratcha.global.canton.network.digitalasset.com

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hardcoding definitely won't work, so either you get rid of the need to set it or you need to set it from pulumi

pathPrefix: /registry
throttleAcrossAllEndpointsAllIps:
withinIntervalSeconds: 60
maxRequestsBeforeHttp429: 200

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what number do we currently get on the prod clusters? we need to make sure this is not lower than what people already expect.

maxTokens: 10
tokensPerFill: 5
fillInterval: 60s
/registry/allocations/v1:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're adding this to v0-acs.yaml. probably want a separate file instead, this is not about acs endpoints

throttleAcrossAllEndpointsAllIps:
maxRequestsBeforeHttp429: 0
withinIntervalSeconds: 60
tokenRegistry:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're enabling a global cloud armor limit but cloud armor is disabled. I was expecting we try to solve this in istio for now, is that not an option?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try setting the limit low enough that you can easily trigger it during testing and see that you trigger it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose Scan:: registry/token: Change istio authorization policies to allow traffic to the registry endpoints

3 participants