the-cloudkeeper-project / cloudkeeper

EGI AppDB <-> CMF synchronization utility
Other
2 stars 8 forks source link

'ad:traffic_in' and 'ad:traffic_out' json dicts are changed/rubyfied #78

Open gwarf opened 5 years ago

gwarf commented 5 years ago

Hello, Following some debugging for https://github.com/EGI-Foundation/cloud-info-provider/issues/149 it seems that information about ad:traffic_our and ad:traffin_in image attributes is rubyfied. In https://vmcaster.appdb.egi.eu/store/vappliance/scipion.v1.0/image.list it's expressed as a JSON dict:

"ad:traffic_in": [
  {
    "ad:net_protocol": "tcp",
    "ad:net_port": "8000:8000",
    "ad:net_range": "0.0.0.0/0"
  }
],

And once it went through cloudkeeper and cloudkeeper-os it's recorded like this in OpenStack, as a string of a somewhat rubyfied hash (with weird : before the keys):

[{:“ad:net_protocol”=>“tcp”, :“ad:net_port”=>“8000:8000", :“ad:net_range”=>“0.0.0.0/0"}]

Looking at https://github.com/the-cloudkeeper-project/cloudkeeper-os/blob/master/cloudkeeper_os/utils.py#L90 it seems that it more something that may be done by cloudkeeper itself.

Could you please have a look?

gwarf commented 5 years ago

Pinging @Pansanel, @mhaggel and @enolfc.

amarthadan commented 5 years ago

Cloudkeeper < 2.x sends all the attributes of an appliance as a map String -> String. It doesn't take into consideration that some values might have additional structure. This won't happen with Cloudkeeper > 2.x since it's not sending all the attributes like before.

Is this something worth fixing in 1.x branch?

enolfc commented 5 years ago

Cloudkeeper < 2.x sends all the attributes of an appliance as a map String -> String. It doesn't take into consideration that some values might have additional structure. This won't happen with Cloudkeeper > 2.x since it's not sending all the attributes like before.

but still the attributes will be available to the backend, no?

Is this something worth fixing in 1.x branch?

Really depends on the timeline of the cloukeeper-os I believe. For the time being we are adding a fix at the cloud-info-provider so not really urgent

amarthadan commented 5 years ago

but still the attributes will be available to the backend, no?

No, not all of them. But if some attribute is needed on the backend I can extend the proto definition and add it. Just let me know what you need.

gwarf commented 5 years ago

Is it possible to know what is not planned to be made available? What is the reason behind it, potential security-related problem?

amarthadan commented 5 years ago

These are the attributes available for appliance:

and these two from image list section:

The reason was restrictions on image tags in OpenNebula (character restrictions) and AWS (length resctriction). In order to use Cloudkeeper on all cloud platforms in the same way this is the compromise we had to make.

enolfc commented 5 years ago

I would expect the restrictions to be handled by the backends and keep the core as flexible as possible.

amarthadan commented 5 years ago

I figured that having a protocol with only clearly specified values is better then pushing all values from image list as a non-transparent map. The protocol can be easily extended (with backward compatibility) if some values are missing.

But if you insist on pushing all the values it can be brought back.

Anyway, that's a different issue. Can I close this issue as #wontfix?