Skip to content

Update hardcoded URL in provider.yaml to allow flexibility with regard to namespace#184

Merged
susanshi merged 1 commit intonotaryproject:mainfrom
lee0c:lecattar/helm-provider-url
May 5, 2022
Merged

Update hardcoded URL in provider.yaml to allow flexibility with regard to namespace#184
susanshi merged 1 commit intonotaryproject:mainfrom
lee0c:lecattar/helm-provider-url

Conversation

@lee0c
Copy link
Contributor

@lee0c lee0c commented May 4, 2022

At current the provider.yaml file in the Helm chart sets a ratify URL that ties it to the default namespace even if the chart is deployed in another namespace. This should make it align with any namespace.

@susanshi
Copy link
Collaborator

susanshi commented May 4, 2022

Hi @lee0c, the changes looks good to me. A question for my learning, does gatekeeper depend on the ratify provider url? How does gatekeeper find out the provider url to send the request?

1 similar comment
@susanshi
Copy link
Collaborator

susanshi commented May 4, 2022

Hi @lee0c, the changes looks good to me. A question for my learning, does gatekeeper depend on the ratify provider url? How does gatekeeper find out the provider url to send the request?

@susanshi
Copy link
Collaborator

susanshi commented May 4, 2022

@lee0c , could you also provide the sample url when fullnameOverride is empty vs provided. Just wanted to review the updated url. thanks!

1 similar comment
@susanshi
Copy link
Collaborator

susanshi commented May 4, 2022

@lee0c , could you also provide the sample url when fullnameOverride is empty vs provided. Just wanted to review the updated url. thanks!

@lee0c
Copy link
Contributor Author

lee0c commented May 4, 2022

A question for my learning, does gatekeeper depend on the ratify provider url? How does gatekeeper find out the provider url to send the request?

Yup, so Gatekeeper defines a CRD of kind Provider which Ratify needs to create in order for Gatekeeper to be aware of it as an external data provider. The YAML file I've edited here is Ratify's Provider resource which Gatekeeper will notice on creation/update.

@lee0c
Copy link
Contributor Author

lee0c commented May 4, 2022

provide the sample url when fullnameOverride is empty vs provided

@susanshi (should've tagged you in previous commenta s well, apologies) - if fullNameOverride is set to something other than empty string, ratify.fullname will be fullNameOverride. Otherwise, it will work out to be either release name or a combination of release name and nameOverride.

The important part is that the URL matches the name of the Service being deployed, which is why the service spec is updated as well.

For example, I've templated it out here with --set fullnameOverride=testing and got a URL of url: "http://testing.ratify-system:6001/ratify/gatekeeper/v1/verify". If I run it without that --set, templating gives me a URL of url: "http://release-name-ratify.ratify-system:6001/ratify/gatekeeper/v1/verify". release-name-ratify is due to templating - since it's not a real release, .Release.Name is set to release-name and the _helpers.tpl file evaluates fullname to be release name & "ratify" (which is ratify.name). You can see all this logic in helpers.tpl

Copy link
Collaborator

@susanshi susanshi left a comment

Choose a reason for hiding this comment

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

thanks for the walking through the details @lee0c!

@susanshi
Copy link
Collaborator

susanshi commented May 4, 2022

@lee0c, i have approved these changes, is it ok to leave open for a day incase others have additional comments?

@lee0c
Copy link
Contributor Author

lee0c commented May 4, 2022

@susanshi Sure thing, this one isn't urgent in any way. Close when y'all are ready :)

@susanshi susanshi merged commit a4cca38 into notaryproject:main May 5, 2022
bspaans pushed a commit to bspaans/ratify that referenced this pull request Oct 17, 2023
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.

2 participants