search

Azure / open-service-broker-azure

The Open Service Broker API Server for Azure Services
https://osba.sh
MIT License
248 stars 100 forks source link

Fix icons #669

Closed zhongyi-zhang closed 5 years ago

zhongyi-zhang commented 5 years ago

Fix #666.

zhongyi-zhang commented 5 years ago

Can anyone help verify the fix? @arapulido @alberttwong could you? Not quite sure if it works for you.

alberttwong commented 5 years ago

I used https://raw.githubusercontent.com/zhongyi-zhang/open-service-broker-azure/icon/contrib/openshift/osba-os-template.yaml and it didn't work.

zhongyi-zhang commented 5 years ago

Oh, sorry I shall pulish a test image for your try. That template you used installs from a certain published image. (It is microsoft/azure-service-broker:v1.0.1 as you can see.) Now it is too late in my time zone. I'll try to publish the test image tomorrow. And will be back here if done.

zhongyi-zhang commented 5 years ago

OSBA image publish pipeline can only be kicked after merging the PR into master. So I am using a personal repo for the test image. Please have a try if you don't mind: zzy940502/azure-service-broker:canary.

alberttwong commented 5 years ago

So what do I put for the deployment command in OCP?

oc process -f https://raw.githubusercontent.com/Azure/open-service-broker-azure/master/contrib/openshift/osba-os-template.yaml  \
   -p ENVIRONMENT=AzurePublicCloud \
   -p AZURE_SUBSCRIPTION_ID=x-a04c-4f13-8180-d \
   -p AZURE_TENANT_ID=x-90fa-482b-b2a7-7de884fd2ff7 \
   -p AZURE_CLIENT_ID=d61ffa2e-x-4feb-8353-x \
   -p AZURE_CLIENT_SECRET=5g7CihT/x+8= \
   | oc create -f -
zhongyi-zhang commented 5 years ago

@alberttwong find this line image: microsoft/azure-service-broker:v1.0.1 in the template you used, change to zzy940502/azure-service-broker:canary.

arapulido commented 5 years ago

With the testing image, I get the new URLs for the images correctly, but those don't render fine. For cosmosdb.svg icon, it points to this image. But that one doesn't render correctly in Firefox or Chrome.

zhongyi-zhang commented 5 years ago

@arapulido yeah, for direct browser opening, the xml declaration makes it only show the raw. But Github still displays it correctly: https://github.com/MicrosoftDocs/azure-docs/blob/master/articles/media/index/cosmosdb.svg. Maybe it requires extra renderer? If so, can it be fixed in the UI provider side? Or, we can just convert the svg to png and keep them in this repo. (This is not ideal IMO :()

arapulido commented 5 years ago

@zhongyi-zhang that's not the reason. As you can see with this other svg the image renders correctly when opening with a browser. The problem with your images is the tag in the first line. If you remove that one, it shows correctly in the browser

zhongyi-zhang commented 5 years ago

Just found that appending ?sanitize=true can do the trick. Also updated the test image. Please have another try,

arapulido commented 5 years ago

@zhongyi-zhang they all work well except the mysql ones, that is missing (404): https://raw.githubusercontent.com/MicrosoftDocs/azure-docs/9eb1f875f3823af85e41ebc97e31c5b7202bf419/articles/media/index/MySQL.svg?sanitize=true

zhongyi-zhang commented 5 years ago

@arapulido thanks for the feedback! Fixed now. @norshtein, this PR can be merged after getting an approval.